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-1780: Set up SAMBAH chain to skip concise if extend is requested but not concatenation. #685

Merged
merged 4 commits into from
Jan 30, 2025

Conversation

chris-durbin
Copy link
Contributor

Jira Issue ID

HARMONY-1780

Description

ASDC asked for us to change the behavior of the SAMBAH chain to skip calling podaac-concise if the user requested extend, but did not specify concatenation. They also specified a few other AC that are already working as expected prior to this PR.

Note that the extend parameter is overloaded - it can be treated as a boolean ('true' or 'false') or as a comma-separated list of dimensions to use as the extend dimensions.

Local Test Steps

See the behavior requested in the AC and make sure that all of it passes.

  1. Add batchee,stitchee,podaac-l2-subsetter,podaac-concise to your LOCALLY_DEPLOYED_SERVICES in .env
  2. Add the following to your .env:
BATCHEE_IMAGE=ghcr.io/nasa/batchee:1.3.0
STITCHEE_IMAGE=ghcr.io/nasa/stitchee:1.6.1
BATCHEE_SERVICE_QUEUE_URLS='["ghcr.io/nasa/batchee:1.3.0,http://sqs.us-west-2.localhost.localstack.cloud:4566/000000000000/batchee.fifo"]'
STITCHEE_SERVICE_QUEUE_URLS='["ghcr.io/nasa/stitchee:1.6.1,http://sqs.us-west-2.localhost.localstack.cloud:4566/000000000000/stitchee.fifo"]'
  1. Test all of the following requests and make sure they meet the AC:
    Custom dimensions, concatenate true - runs all services: http://localhost:3000/C1254854453-LARC_CLOUD/ogc-api-coverages/1.0.0/collections/all/coverage/rangeset?subset=lat(-70%3A30)&subset=lon(-157.5%3A-10)&concatenate=true&extend=foo,bar&maxResults=2

Custom dimensions, no concatenation - skips Concise: http://localhost:3000/C1254854453-LARC_CLOUD/ogc-api-coverages/1.0.0/collections/all/coverage/rangeset?subset=lat(-70%3A30)&subset=lon(-157.5%3A-10)&extend=foo,bar&maxResults=2

No extend, only concatenation - runs all services: http://localhost:3000/C1254854453-LARC_CLOUD/ogc-api-coverages/1.0.0/collections/all/coverage/rangeset?subset=lat(-70%3A30)&subset=lon(-157.5%3A-10)&concatenate=true&maxResults=2

No extend, no concatenation - only runs l2ss-py: http://localhost:3000/C1254854453-LARC_CLOUD/ogc-api-coverages/1.0.0/collections/all/coverage/rangeset?subset=lat(-70%3A30)&subset=lon(-157.5%3A-10)&maxResults=2

Extend true, no concatenation - skips Concise, uses default extendDimensions (will need to look at the operation in the database or print out the operation in the code): http://localhost:3000/C1254854453-LARC_CLOUD/ogc-api-coverages/1.0.0/collections/all/coverage/rangeset?subset=lat(-70%3A30)&subset=lon(-157.5%3A-10)&maxResults=2&extend=true

Extend false, no concatenation - only runs l2ss-py: http://localhost:3000/C1254854453-LARC_CLOUD/ogc-api-coverages/1.0.0/collections/all/coverage/rangeset?subset=lat(-70%3A30)&subset=lon(-157.5%3A-10)&maxResults=2&extend=false

PR Acceptance Checklist

  • Acceptance criteria met
  • Tests added/updated (if needed) and passing
  • Documentation updated (if needed)
  • Harmony in a Box tested? (if changes made to microservices)

@chris-durbin
Copy link
Contributor Author

FYI @danielfromearth and @ank1m so you can verify the behavior matches your expectations.

@ygliuvt
Copy link
Member

ygliuvt commented Jan 29, 2025

Question: When concatenate is true and extend is false (explicitly), the default extend dimension is used for the request currently. i.e. extend is false is ignored. Do we want this behavior or a validation error instead for this case?

@ank1m
Copy link

ank1m commented Jan 29, 2025

Question: When concatenate is true and extend is false (explicitly), the default extend dimension is used for the request currently. i.e. extend is false is ignored. Do we want this behavior or a validation error instead for this case?

concatenate=true and extend=false is effectively L2SS-CONCISE chain, so it's a valid behavior and we probably want it, yes.

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.

…e and concatenate=true, batchee and stitchee will be skipped and only l2ss-py and concise will run.
@chris-durbin
Copy link
Contributor Author

Question: When concatenate is true and extend is false (explicitly), the default extend dimension is used for the request currently. i.e. extend is false is ignored. Do we want this behavior or a validation error instead for this case?

concatenate=true and extend=false is effectively L2SS-CONCISE chain, so it's a valid behavior and we probably want it, yes.

I made the change to only run l2ss-py and concise in this scenario.

@ank1m
Copy link

ank1m commented Jan 29, 2025

Just wanted to make sure that extend=true by default. Because from EDS interface user only has concatenate=true choice, and that implies both concatenate=true and extend=true.
But also if user on EDS unchecks concatenate box, then it means concatenate=false and extend=false.

@chris-durbin
Copy link
Contributor Author

Just wanted to make sure that extend=true by default. Because from EDS interface user only has concatenate=true choice, and that implies both concatenate=true and extend=true. But also if user on EDS unchecks concatenate box, then it means concatenate=false and extend=false.

Yes, that's the behavior - tested by this case:
No extend, only concatenation - runs all services: http://localhost:3000/C1254854453-LARC_CLOUD/ogc-api-coverages/1.0.0/collections/all/coverage/rangeset?subset=lat(-70%3A30)&subset=lon(-157.5%3A-10)&concatenate=true&maxResults=2

@chris-durbin chris-durbin merged commit 60deaf5 into main Jan 30, 2025
5 checks passed
@chris-durbin chris-durbin deleted the HARMONY-1780 branch January 30, 2025 18: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.

4 participants