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

adding instance_format to variable query #60

Closed
wants to merge 0 commits into from

Conversation

briannapagan
Copy link
Collaborator

Following the conversation here: #59
NASA GES DISC is releasing public zarr stores for specific variables of collections. The only way to discover what available zarr stores at variable level is via the CMR parameter instance_format. This simple PR adds this as a search parameter in VariablesQuery.

cmr/queries.py Outdated
"""

if not instance_format:
return self
Copy link
Contributor

@sliu008 sliu008 Jun 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably be better to do this instead

if instance_format:
    self.params['instance_format'] = instance_format

return self

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agreed, it's a minor difference, but slightly preferred

@@ -85,3 +85,10 @@ def bearer_test_token(self):

self.assertIn("Authorization", query.headers)
self.assertEqual(query.headers["Authorization"], "Bearer 123TOKEN")

def test_archive_center(self):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
def test_archive_center(self):
def test_instance_format(self):

cmr/queries.py Outdated
"""

if not instance_format:
return self
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agreed, it's a minor difference, but slightly preferred

@frankinspace
Copy link
Collaborator

Also please add an entry to the changelog

@chuckwondo
Copy link
Collaborator

@briannapagan, do you have time to address the change requests and add the appropriate changelog entry, so we can get this into the next release?

@chuckwondo
Copy link
Collaborator

@briannapagan, are you able to address the changes requested on the PR, including adding a changelog entry? There have been a few other PRs merged into the develop branch since you opened this one, so you'll have to get your branch up to date with the develop branch as well.

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.

4 participants