Skip to content
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

Add post_baseUrl to delete on post, changes to logging #1050

Merged
merged 1 commit into from
Jun 3, 2024

Conversation

reidsunderland
Copy link
Member

I had made some changes that I forgot to push for #1042

The most important one is adding the post_baseUrl to the delete on post, so it doesn't propagate to the downloading component.

Copy link

Test Results

220 tests   212 ✅  16s ⏱️
  1 suites    8 💤
  1 files      0 ❌

Results for commit 472427e.

@petersilva
Copy link
Contributor

petersilva commented May 15, 2024

confused... if you are in a poll, you should just set baseUrl ... why do you need post_baseUrl?

@reidsunderland
Copy link
Member Author

Because it eventually gets overridden later in the code. #951

For this plugin, pollUrl should be set to https://cmr.earthdata.nasa.gov/search/granules.umm_json in the config file. Because post_baseUrl is not set in the config, self.o.post_baseUrl gets set to the value of pollUrl. (Here's an example config: https://github.com/MetPX/sarracenia/blob/development/sarracenia/examples/poll/nasa_cmr_podaac.conf).

For example, if the poll creates a message with msg['new_baseUrl'] = "https://downloadfromhere.com" and msg['baseUrl'] = "https://downloadfromhere.com" and does not set msg['post_baseUrl']...

Once the list of messages is returned from the poll, they get filtered in the flow algorithm. filter() calls updateFieldsAccepted(), which calls updatePaths(). And updatePaths has this logic:

# post_baseUrl option set in msg overrides other possible options
if 'post_baseUrl' in msg:
baseUrl_str = msg['post_baseUrl']
elif options.post_baseUrl:
baseUrl_str = options.variableExpansion(options.post_baseUrl, msg)
else:
if 'baseUrl' in msg:
baseUrl_str = msg['baseUrl']
else:
logger.error('missing post_baseUrl setting')
return

If I don't set msg['post_baseUrl'], then we'd go into the elif case, because options.post_baseUrl is set (to the value of pollUrl from the config). This causes updatePaths to set msg['new_baseUrl'] to the value of options.post_baseUrl (== pollUrl).

So now the message has been updated to be this:

msg['new_baseUrl'] = "https://whatever_I_set_pollUrl_to_in_the_config.com" and msg['baseUrl'] = "https://downloadfromhere.com"

Then finally, we get to this part of the code:

for m in self.worklist.ok:
if ('new_baseUrl' in m) and (m['baseUrl'] !=
m['new_baseUrl']):
m['old_baseUrl'] = m['baseUrl']
m['_deleteOnPost'] |= set(['old_baseUrl'])
m['baseUrl'] = m['new_baseUrl']

new_baseUrl is in m, and m['baseUrl'] and m['new_baseUrl'] are not equal, so this part of the code sets the value of baseUrl to whatever is in new_baseUrl.

Now we have: msg['new_baseUrl'] = "https://whatever_I_set_pollUrl_to_in_the_config.com" and msg['baseUrl'] = "https://whatever_I_set_pollUrl_to_in_the_config.com".


There's a little bit of history missing from #951. This plugin used to work (see original code and examples here f1d7697) because I was leaving both pollUrl and post_baseUrl undefined in the config file. In that situation, we'd fall into the else on line 796 and updatePaths would not change the 'baseUrl' and 'new_baseUrl' fields in the message.

But something changed between October 11, 2023 and now where the poll won't start if pollUrl is not set in the config file. I used to be able to set self.o.pollUrl in the plugin's init and it would work. (I did that because I figured it was easier for the user to just define the plugin in the config and not have to worry about setting pollUrl themself, because it's always the same URL)

@reidsunderland
Copy link
Member Author

I also tried setting self.o.pollUrl and self.o.post_baseUrl in the poll, but that doesn't propagate to other parts of the code, because I think it has a deepcopy of the options, not the original instance used in other parts.

@petersilva
Copy link
Contributor

Can you try putting self.o.add_option( 'pollUrl' , type='str', default='thevalueUwant' ) in the init ...
and see if that works. I saw you experimented with just setting it in the plugin, but I'm wondering if using add_option will work better.

@petersilva petersilva merged commit f98f0e9 into development Jun 3, 2024
28 of 35 checks passed
@petersilva petersilva deleted the nasa_cmr_poll_fix branch June 6, 2024 21:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants