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

HARMONY-1762: Switch to passing variables in the query parameters #85

Merged
merged 1 commit into from
Jun 10, 2024

Conversation

chris-durbin
Copy link
Contributor

Jira Issue ID

HARMONY-1762

Description

Change to pass the variables in the query parameters using parameter_vars so that we can support requests with 4000+ variables.

Local Test Steps

I tested with the intro_tutorial notebook and modifying the request to use a bounding box instead of a shapefile (the request_as_url and request_as_curl methods do not work with a shapefile).

Add a couple print statements to the notebooks to see what the URL looks like:
print(harmony_client.request_as_url(request))
print(harmony_client.request_as_curl(request))

Make sure you see it uses parameter_vars and the variable is included in the query parameters.

Also test removing the variables altogether and make sure it passes variable=all.

It would be good to test with a large number of variables as well if someone has a good request for that.

PR Acceptance Checklist

  • Acceptance criteria met
  • Tests added/updated (if needed) and passing
  • Documentation updated (if needed)

…order to support a larger number of variables than could be specified in the URL.
Copy link
Member

@ygliuvt ygliuvt left a comment

Choose a reason for hiding this comment

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

Tested a couple notebooks and the harmony requests still works.

@chris-durbin chris-durbin merged commit 51ae0fd into main Jun 10, 2024
9 of 11 checks passed
@chris-durbin chris-durbin deleted the harmony-1762 branch June 10, 2024 14:02
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.

3 participants