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.

Sign up for GitHub

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

Jump to bottom

fix cds-coordinator-metadata-query-disabled #16488

Merged
merged 4 commits into from
May 22, 2024
Merged

fix cds-coordinator-metadata-query-disabled #16488

merged 4 commits into from
May 22, 2024

Conversation

kgyrtkirk
Copy link
Member

@kgyrtkirk kgyrtkirk commented May 22, 2024 •
edited
Loading

fixes the issue with the newly enabled cds-coordiantor-metadata-query-disabled split

  • configures to use prepopulated-data environment things to configure S3 for access
  • this is needed because these tests use a dataset which is loaded from s3
  • also undoes the previous fix of setting the aws region explicitly as this is a more complete solution - and configuring prepopulated-data also sets the region; so that's not needed anymore

@github-actions github-actions bot added the GHA label May 22, 2024
@kgyrtkirk kgyrtkirk mentioned this pull request May 22, 2024
Closed
findingrish
findingrish approved these changes May 22, 2024
View reviewed changes
Copy link
Contributor

@findingrish findingrish left a comment

Choose a reason for hiding this comment

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

LGTM!

gianm
gianm approved these changes May 22, 2024
View reviewed changes
Copy link
Contributor

@gianm gianm left a comment

Choose a reason for hiding this comment

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

It seems like this helps the test, but why? Please write a PR description prior to merging, so it shows up in the commit log.

@kgyrtkirk kgyrtkirk merged commit 44ea4e1 into apache:master May 22, 2024
87 checks passed
@kgyrtkirk
Copy link
Member Author

Thank you @gianm and @findingrish for taking a look;
I've described the issue/reasons behind the changes!

ektravel pushed a commit to ektravel/druid that referenced this pull request May 29, 2024
@kgyrtkirk @ektravel
Fix cds-coordinator-metadata-query-disabled (apache#16488)
c4322f6 
fixes the issue with the newly enabled `cds-coordiantor-metadata-query-disabled` [split](apache#16468)
* configures to use `prepopulated-data` environment things to configure `S3` for access 
* this is needed because these tests use a [dataset which is loaded from s3](https://github.com/apache/druid/blob/master/integration-tests/docker/test-data/cds-coordinator-metadata-query-disabled-sample-data.sql)
* also undoes the previous [fix](apache#16469) of setting the aws region explicitly as this is a more complete solution - and configuring `prepopulated-data` also sets the region; so that's not needed anymore
@kfaraz kfaraz added this to the 31.0.0 milestone Oct 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@findingrish findingrish findingrish approved these changes

@gianm gianm gianm approved these changes

Assignees
No one assigned
Labels
GHA
Projects
None yet
Milestone
31.0.0
Development

Successfully merging this pull request may close these issues.
4 participants
@kgyrtkirk @gianm @findingrish @kfaraz