-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow subdirectories for add & delete granules #20
Conversation
Codecov Report
@@ Coverage Diff @@
## main #20 +/- ##
==========================================
+ Coverage 93.51% 94.16% +0.64%
==========================================
Files 4 4
Lines 833 1028 +195
==========================================
+ Hits 779 968 +189
- Misses 54 60 +6
Flags with carried forward coverage won't be shown. Click here to find out more.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we're defining multiple layers with the same config file, it might make sense to have the files in subdirectories per layer this allows keeping the subdir specified in argument or keeping the path from the geoserver granule location Note! Deleting files with this might fail if the path in geoserver is different than path on host server!
4b7a305
to
acb190b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a couple of comments that would simplify the configs a bit for cases where sub directories don't need to be handled.
I'll also push some additional tests for georest.utils.convert_file_path()
to this branch in a moment when I've finalized them.
georest/__init__.py
Outdated
@@ -286,7 +286,7 @@ def add_file_to_mosaic(config, fname_in, filesystem='posix'): | |||
This function wraps some boilerplate around adding a granule to a layer. | |||
|
|||
""" | |||
fname = utils.convert_file_path(config, fname_in) | |||
fname = utils.convert_file_path(config, fname_in, keep_subpath=config["keep_subpath"]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could also be
fname = utils.convert_file_path(config, fname_in, keep_subpath=config["keep_subpath"]) | |
fname = utils.convert_file_path(config, fname_in, keep_subpath=config.get("keep_subpath", False)) |
so forgetting the extra option from the config doesn't cause a crash.
georest/__init__.py
Outdated
|
||
|
||
def _delete_files_from_fs(config, gs_location): | ||
delete_files = config.get("delete_files", False) | ||
if not delete_files: | ||
return | ||
fs_path = utils.convert_file_path(config, gs_location, inverse=True) | ||
fs_path = utils.convert_file_path(config, gs_location, inverse=True, keep_subpath=config["keep_subpath"]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fs_path = utils.convert_file_path(config, gs_location, inverse=True, keep_subpath=config["keep_subpath"]) | |
fs_path = utils.convert_file_path(config, gs_location, inverse=True, keep_subpath=config.get("keep_subpath", False)) |
@@ -85,9 +85,12 @@ def _write_property_zip(prop_paths, zip_path): | |||
return zip_path | |||
|
|||
|
|||
def convert_file_path(config, file_path, inverse=False): | |||
def convert_file_path(config, file_path, inverse=False, keep_subpath=False): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apparently I haven't had a test case for this function, but we could add something now to test_utils.py
. I'll write some and push them to this branch in a moment.
@ritvje do you think the cases I added for |
Looks good. Your version of the inverse option of |
47167d0
to
2f8d905
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good, merging.
Add a config option that allows keeping the subpath given in arguments when adding granules.
E.g. if config file has
if the filepath for
add_granule.py
is given asfivih_3067/20230926_1300_fivih_ppi-0.3_optclass.tif
, the filepath is interpreted as/smartmet/radar/geotiff/radar-optclass/fivih_3067/20230926_1300_fivih_ppi-0.3_optclass.tif