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

Port #49964 to master #57111

Merged
merged 3 commits into from May 26, 2021
Merged

Conversation

alexey-zhukovin
Copy link
Contributor

What does this PR do?

Port #49964 to master

@alexey-zhukovin alexey-zhukovin added Needs-Testcase PR needs test cases written, or the issue is about a bug/feature that needs test cases master-port labels May 6, 2020
@alexey-zhukovin alexey-zhukovin requested a review from a team as a code owner May 6, 2020 17:20
@ghost ghost requested review from twangboy and removed request for a team May 6, 2020 17:20
@sagetherage sagetherage added this to PR needs merge to master in PRs to port to master via automation May 12, 2020
waynew and others added 3 commits May 20, 2021 10:37
I had considered doing something with regex like `\d+(\.\d+)*`, then I
realized that there are other valid version strings that perhaps do not
contain numerics separated by `.`, so this option is a bit more flexible
as far as what versions nginx can return.

The major weakness here is that it does require nginx to return the
version on the first line, and to use a `/` separator before the version
itself.

One potential alternative would be to use the regex
`r'(\w+\.(\w+\.?)*)'` and get groups()[0] from that, however that
*would* require at least a decimal point.

There's a number of other options that one could use here, however,
we're currently simply banking on nginx always returning the actual
version prefixed with a forward slash.
@waynew waynew removed Needs-Testcase PR needs test cases written, or the issue is about a bug/feature that needs test cases has-failing-test labels May 20, 2021
@Ch3LL Ch3LL merged commit 09cc2f3 into saltstack:master May 26, 2021
PRs to port to master automation moved this from PR needs merge to master to PR merged May 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

None yet

7 participants