-
Notifications
You must be signed in to change notification settings - Fork 536
iserver-test: Overhaul upgrade config #19020
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
base: main
Are you sure you want to change the base?
Conversation
🤖 GitHub commentsExpand to view the GitHub comments
Just comment with:
|
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.
nit but code looks good, thanks!
to, err := parseLazyRolloverExceptionVersionRange(e.To) | ||
if err != nil { | ||
return upgradeTestConfig{}, fmt.Errorf( | ||
"failed to parse lazy-rollover-exception version '%s': %w", e.From, err) |
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.
"failed to parse lazy-rollover-exception version '%s': %w", e.From, err) | |
"failed to parse lazy-rollover-exception to version '%s': %w", e.To, err) |
nit: typo
from, err := parseLazyRolloverExceptionVersionRange(e.From) | ||
if err != nil { | ||
return upgradeTestConfig{}, fmt.Errorf( | ||
"failed to parse lazy-rollover-exception version '%s': %w", e.From, err) |
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.
"failed to parse lazy-rollover-exception version '%s': %w", e.From, err) | |
"failed to parse lazy-rollover-exception from version '%s': %w", e.From, err) |
nit: clarity
# ``` | ||
# | ||
# Both major and patch positions are not given the special wildcard (x) because | ||
# it is unlikely to be useful in our use cases. If due to some mistake, the |
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 due to some mistake
non blocker: I wonder if it is just easier to ban it outright to avoid any misuse.
Motivation/summary
Closes #17314.
Add a new way of defining lazy rollover exceptions.
How to test these changes
Run workflow: https://github.com/elastic/apm-server/actions/runs/18378244354.