-
Notifications
You must be signed in to change notification settings - Fork 18
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
ART-6684 Get latest nightly status #179
Conversation
if status_code != 200: | ||
logger.error('Server responded with status code %s', status_code) | ||
|
||
return response.json()['tags'][0]['name'] |
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.
This will fail, especially when status_code is not 200.
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.
This should be fixed now
artbotlib/nightly_color.py
Outdated
@@ -62,6 +62,43 @@ def get_failed_jobs(release_url, release_browser) -> str: | |||
return payload | |||
|
|||
|
|||
def get_latest_nightly_name(release_brower, release_stream) -> str: | |||
url = f"https://{release_brower}.ocp.releases.ci.openshift.org/api/v1/releasestream/{release_stream}/tags" |
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.
Typo. Maybe rename release_brower
to arch
?
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.
As per Ximin's comment, I have removed the option to use architectures other than amd64
artbotlib/nightly_color.py
Outdated
|
||
response = requests.get(url) # query API to get list of nightly tags | ||
status_code = response.status_code | ||
if status_code != 200: |
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.
variable name is not needed, can call response.status_code
directly here
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.
Thank you, updated
artbotlib/regex_mapping.py
Outdated
@@ -307,6 +307,13 @@ def map_command_to_regex(so, plain_text, user_id): | |||
"user_id": True, | |||
"example": "Watch https://amd64.ocp.releases.ci.openshift.org/releasestream/4.13.0-0.ci/release/4.13.0-0.ci-2022-12-19-111818" | |||
}, | |||
{ | |||
"regex": r"^Alert ?(if|when|on)? latest ? (?P<release_browser>[\w]+)? ?(?P<version>\d+.\d+) ?(stops being blue|fails|is rejected|is red|is accepted|is green)?$", |
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.
I think the regex could use some work. The ?
are placed a bit randomly.
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.
I have modified the pattern according to Ximin's suggestion, which simplified the regex quite a bit. Hopefully it is correct now
artbotlib/regex_mapping.py
Outdated
"flag": re.I, | ||
"function": latest_nightly_color_status, | ||
"user_id": True, | ||
"example": "Alert when latest s390x 4.16 is accepted" |
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.
I prefer the pattern "watch latest 4.16", no need to match those extra words
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.
Fixed
artbotlib/nightly_color.py
Outdated
if release_browser is None: # Assume amd64 if no arch given | ||
release_browser = "amd64" | ||
|
||
if release_browser == "amd64": |
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.
we usually only care about amd64, other arch like s390x don't have test jobs so won't need to wait.
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.
I see, I have updated it accordingly
abb2e93
to
1b30c6e
Compare
@adobes1: The following test failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
/lgtm |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: joepvd, Ximinhan The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
No description provided.