-
Notifications
You must be signed in to change notification settings - Fork 2
Upgrade/improve product exists logic #68
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
Conversation
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.
Generally looks good to me -- I've made minor suggestions to do with formatting.
Co-authored-by: Caitlin Adams <[email protected]>
Co-authored-by: Caitlin Adams <[email protected]>
Co-authored-by: Caitlin Adams <[email protected]>
Co-authored-by: Caitlin Adams <[email protected]>
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.
Hi Alex, thanks for adding the capability to exit early if the static layers exist on S3.
Two suggestions:
- Even if
--scene_data_source CDSE, can't we still useasf_searchoutside the if else block to check whether the static layers exist on S3? If the user hasn't provided ASF credentials, then I understand we can revert to our current fallback of downloading the scene with CDSE. - For exiting early, it is unclear to me how this currently happens. Is it that the
burst_id_listwill be empty? If this is the case, maybe it's worth adding a more explicit exit statement.
Thanks Aman:
|
I guess my point here was that even if we are using the CDSE to download the scene, there's no reason we can't use the ASF to check whether the static layers exist on S3 first, right? |
Yeah you're right it's probably just good practice to do that for all scenario's and download the scene if they can't be found and get the burst list from the SLC. Will update |
Would the user have to supply both ASF and CDSE credentials for this to work? |
Not for the search, I don't think credentials are needed. Just the download. Will make sure my logic accounts for that |
|
@caitlinadams and @geoscience-aman I have added the logic to query the ASF for bursts in the scene even if the CDSE is set as the data source. Are you able to review? |
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.
Looks good to me, thanks Alex!
Main Changes
make_existing_productsis set)--scene_data_source ASFis set, as the burst_id's for a given scene cannot be queried from the CDSE (It seems the burst_id in the format oft012_025211_iw2is an ASF thing. Thus,--scene_data_source CDSEwill still download the scene before checking if the burst products existRTC_S1_STATIClayers. We can now run a large time period with little overhead to ensure all of the required static layers are created.Other