Skip to content

Conversation

ywangd
Copy link
Member

@ywangd ywangd commented Jul 17, 2025

Also add test to ensure the file has at least one entry for each region so that it is easy to spot missing regions in future upgrades.

Relates: #131050
Resolves: #131392

Also add test to ensure the file has at least one entry for each region
so that it is easy to spot missing regions in future upgrades.

Relates: elastic#131050
@ywangd ywangd requested a review from DaveCTurner July 17, 2025 01:48
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-distributed-coordination (Team:Distributed Coordination)

@elasticsearchmachine elasticsearchmachine added the Team:Distributed Coordination Meta label for Distributed Coordination team label Jul 17, 2025
@ywangd
Copy link
Member Author

ywangd commented Jul 17, 2025

Label this as >non-issue since it is categorically coverred by #131050

Copy link
Contributor

@nicktindall nicktindall left a comment

Choose a reason for hiding this comment

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

Nice safeguard going forward!

Copy link
Contributor

@DaveCTurner DaveCTurner left a comment

Choose a reason for hiding this comment

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

Hang on (I'll explain why in a sec)

Copy link
Contributor

@DaveCTurner DaveCTurner left a comment

Choose a reason for hiding this comment

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

Actually hmm this is tricky.

The RegionFromEndpointGuesser is there for bwc with the pre-8.19 behaviour in which you could omit the region, but my thinking was that brand-new regions don't need this bwc. Moreover if we try and keep the list up to date we'll have to keep on upgrading the AWS SDK rather more often than we would otherwise.

However on reflection I think we have to do this anyway because I suspect the v1 SDK will pick up these new regions automatically, so a user on 8.18 in the future may have working region-free config that won't work on upgrade to the v2 SDK unless we keep the v2 SDK up to date.

So, ok to proceed, but some concerns here about the implications.

@ywangd
Copy link
Member Author

ywangd commented Jul 17, 2025

Thanks for sharing the thoughts, David. Very helpful. I guess we probably can still call this best-effort to provide v1 users better upgrading experience, i.e. we strive to not break existing "region free" configs, but it may not be done continiously for every minor sdk version and the fallback (and actually the right solution) for users is to specify the region.

@ywangd
Copy link
Member Author

ywangd commented Jul 17, 2025

@elasticmachine update branch

@ywangd ywangd added auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) auto-backport Automatically create backport pull requests when merged labels Jul 17, 2025
elasticsearchmachine and others added 7 commits July 17, 2025 22:38
The new attribute generated by MV_EXPAND should remain in the original position. The projection added by ProjectAwayColumns does not respect the original order of attributes.

Make ProjectAwayColumns respect the order of attributes to fix this.
* ES|QL categorize options

* refactor options

* fix serialization

* polish

* add verfications

* better test coverage + polish code

* better test coverage + polish code
This PR migrates legacy rest tests in the x-pack autoscaling module
It's already part of the path parts, it's not useful to duplicate it in query
parameters.
@elasticsearchmachine elasticsearchmachine merged commit fd971e8 into elastic:main Jul 17, 2025
33 checks passed
@ywangd ywangd deleted the es-131392-fix branch July 17, 2025 14:15
ywangd added a commit to ywangd/elasticsearch that referenced this pull request Jul 17, 2025
Also add test to ensure the file has at least one entry for each region
so that it is easy to spot missing regions in future upgrades.

Relates: elastic#131050 Resolves: elastic#131392
@elasticsearchmachine
Copy link
Collaborator

💚 Backport successful

Status Branch Result
9.1
8.19

elasticsearchmachine pushed a commit that referenced this pull request Jul 17, 2025
Also add test to ensure the file has at least one entry for each region
so that it is easy to spot missing regions in future upgrades.

Relates: #131050 Resolves: #131392
elasticsearchmachine pushed a commit that referenced this pull request Jul 18, 2025
Also add test to ensure the file has at least one entry for each region
so that it is easy to spot missing regions in future upgrades.

Relates: #131050 Resolves: #131392

Co-authored-by: Elastic Machine <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Automatically create backport pull requests when merged auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) :Distributed Coordination/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs >non-issue Team:Distributed Coordination Meta label for Distributed Coordination team v8.19.1 v9.1.1 v9.2.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CI] S3ServiceTests testGetClientRegionFromEndpointSettingGuess failing
10 participants