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-1721: Switch from using GET to a POST in all OGC coverages calls to harmony #81

Merged
merged 5 commits into from
Mar 14, 2024

Conversation

chris-durbin
Copy link
Contributor

Jira Issue ID

HARMONY-1721

Description

harmony-py was using a GET for all calls to harmony, but this was causing 413 issues when trying to use a long list of granuleIds because of URL length limits. This PR switches to use a POST with multipart/form-data (we were already doing this when a shapefile was included in the request).

Local Test Steps

Run make test and verify all tests pass.
Run the first request in the examples/tutorial.ipynb notebook. Note that there's a bug in harmony parsing width and height which will be addressed as a separate ticket. So you'll need to comment those out so the cell looks like this:

collection = Collection(id='C1234088182-EEDTEST')

request = Request(
    collection=collection,
    spatial=BBox(-165, 52, -140, 77),
    temporal={
        'start': dt.datetime(2010, 1, 1),
        'stop': dt.datetime(2020, 12, 30)
    },
    variables=['blue_var'],
    max_results=10,
    crs='EPSG:3995',
    format='image/tiff',
    # height=512,
    # width=512
)

request.is_valid()

Verify the request completes successfully. You can also check the harmony logs in the UAT environment to tell that it is using a POST and not a GET.

PR Acceptance Checklist

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

… avoid potential issues with query parameters causing the URL to be too long.
…multipart/form-data since that's the only way harmony accepts POST requests on the OGC coverages API. Also fix a bug with the way dimensions were passed.
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 successfully locally.

Copy link
Contributor

@indiejames indiejames left a comment

Choose a reason for hiding this comment

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

Ran tests and tutorial notebook successfully

if key == 'granuleId' and isinstance(values, list):
# Include all granuleId values in a single file boundary as a comma separated
# list to avoid creating too many file boundaries
concatenated_values = ','.join(map(str, values))
Copy link
Contributor

Choose a reason for hiding this comment

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

stupidest join syntax ever

@chris-durbin chris-durbin merged commit 15c09ea into main Mar 14, 2024
11 checks passed
@chris-durbin chris-durbin deleted the harmony-1721 branch March 14, 2024 14:49
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