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

Fixed reg test for cross-region S3 calls #995

Merged
merged 1 commit into from
Feb 14, 2025

Conversation

collado-mike
Copy link
Contributor

The regression test for cross-region S3 calls has been broken for quite a long time, but it never runs in CI anymore, so no one noticed. This updates the catalog creation to take in environment variables, pass in the correct region parameters, and fixes whitespace issues in the output diff.

Comment on lines -45 to -47
curl -H "Authorization: Bearer ${SPARK_BEARER_TOKEN}" -H 'Accept: application/json' -H 'Content-Type: application/json' \
"http://${POLARIS_HOST:-localhost}:8181/api/catalog/v1/config?warehouse=spark_sql_s3_cross_region_catalog"
echo
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like this is being removed because the output will change depending on AWS_REGION_FOR_CROSS_REGION_TEST, is that right?

Rather than removing it, I wonder if we could do some jq magic to make it env-variable-agnostic.

Anyway, it's not an issue to just remove it imo.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, it would be a useful feature to add, but for right now, i just want to make the tests pass :)

@collado-mike collado-mike merged commit 4226713 into apache:main Feb 14, 2025
5 checks passed
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