-
Notifications
You must be signed in to change notification settings - Fork 3.5k
Use centralized version qualifier #16995
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
Use centralized version qualifier #16995
Conversation
To avoid manual invocations specifying the version qualifier for prereleases in this commit we leverage a centralized version of truth for the version qualifier. We also honor the `DRA_BRANCH` BK var for the generated step names (the functionality was already there, it's just a cosmetic improvement).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
| Return the version qualifier from a centralized URL based on branch. For any failure or response apart from 200, | ||
| assume no qualifier (return empty string). | ||
| """ | ||
| url = f"https://storage.googleapis.com/dra-qualifier/{branch}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎉
| if response.status == 200: | ||
| return response.data.decode("utf-8").strip() | ||
| except HTTPError as e: | ||
| pass |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense!
|
|
||
| branch = os.environ["BUILDKITE_BRANCH"] | ||
| # allow manually set version qualifier via BK env vars (should be rarely used, only for testing) | ||
| version_qualifier = os.environ.get("VERSION_QUALIFIER", "") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 for keeping environment var
|
Thank you for the quick review @mashhurs ! Once we've merged it everywhere, we can clean up the docs for using |
Agreed! I was just checking the doc and if we reflect current automated qualifier detection (check the URL if qualifier is missed) and using |
|
💚 Build Succeeded
cc @dliappis |
|
@logstashmachine backport 8.x |
|
@logstashmachine backport 8.17 |
|
@logstashmachine backport 8.16 |
To avoid manual invocations specifying the version qualifier for prereleases in this commit we leverage a centralized version of truth for the version qualifier. We also honor the `DRA_BRANCH` BK var for the generated step names (the functionality was already there, it's just a cosmetic improvement). (cherry picked from commit ac56d99)
To avoid manual invocations specifying the version qualifier for prereleases in this commit we leverage a centralized version of truth for the version qualifier. We also honor the `DRA_BRANCH` BK var for the generated step names (the functionality was already there, it's just a cosmetic improvement). (cherry picked from commit ac56d99)
To avoid manual invocations specifying the version qualifier for prereleases in this commit we leverage a centralized version of truth for the version qualifier. We also honor the `DRA_BRANCH` BK var for the generated step names (the functionality was already there, it's just a cosmetic improvement). (cherry picked from commit ac56d99)
To avoid manual invocations specifying the version qualifier for prereleases in this commit we leverage a centralized version of truth for the version qualifier. We also honor the `DRA_BRANCH` BK var for the generated step names (the functionality was already there, it's just a cosmetic improvement).
|
I am reverting this because testing reveals that even an empty
|
This reverts commit ac56d99. As mentioned in #16995 (comment) testing revealed that even an empty VERSION_QUALIFIER env var currently gets honored for snapshot builds (resulting in an unnecessary empty "-" suffix) so there's a little more prerequisite work needed.





Release notes
[rn:skip]
What does this PR do?
To avoid manual invocations specifying the version qualifier for prereleases in this commit we leverage a centralized version of truth for the version qualifier.
We also honor the
DRA_BRANCHBK var for the generated step names (the functionality was already there, it's just a cosmetic improvement).Why is it important/What is the impact to the user?
Removes the need to specify VERSION_QUALIFIER when building DRA (staging) artifacts entirely.
How to test this PR locally
Can only be tested in BK. Links to be provided:
NOTE This is raised against 9.0 because it's the immediate need, and should be forward ported to
main, as well as backported to all 8.x branches.