Conversation
openshift-ci
bot
added
the
do-not-merge/work-in-progress
|
@anispate could you add a screenshot that demonstrates this UI change? |
|
Hi Aaron, PFA to view the image for the UI Change |
|
@anispate Looks like the style test is failing |
|
Looks good. FWIW, the "osd-" string prefix not technically part of the cluster name. I'm not sure if it's worth the effort to remove it. |
|
I don't think Zabbix is the correct title for this either |
|
/retest |
models.py
Outdated
| from sqlalchemy.orm import relationship | ||
| from sqlalchemy.sql import func as sqlfunc | ||
| from sqlalchemy.ext.hybrid import hybrid_property | ||
| from sqlalchemy import func |
There was a problem hiding this comment.
This was my bad for telling you to add this import, as I didn't realize we already imported func on line 23 (we just rename it to sqlfunc). Could you remove this import and replace your use of func on line 174 with sqlfunc?
Once that's done, remove the [WIP] from this PR's title, and it'll be ready to review.
|
/label tide/merge-method-squash |
openshift-ci
bot
added
the
tide/merge-method-squash
|
@anispate Can you please squash the commits? |
anispate
force-pushed
the
display_cluster_name
branch
from
d8dfcaa to
719b284
Compare
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
questions.py
Outdated
| "Which alerts fire more than once per on-call shift (in the same cluster)?" | ||
| ) | ||
| self._column_names = ["cluster", "name", "namespace", "urgency", "flaps"] | ||
| self._column_names = ["ID", "Cluster", "Alert", "namespace", "urgency", "flaps"] |
There was a problem hiding this comment.
nit: could we stick to the same casing for all column names? e.g. "namespace", "urgency", "flaps" are all lowercase.
There was a problem hiding this comment.
Okay thats a good point. Let me make that change
|
Left one nit. One more nit: Should we call the column "service" instead of "Cluster name", since that is honestly what we're using? Sometimes the service maybe dead man's snitch/Zabbix which isn't really a cluster name. |
|
/lgtm |
anispate
force-pushed
the
display_cluster_name
branch
from
48c9505 to
7ae5d6a
Compare
|
@anispate: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions 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. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: abyrne55, anispate The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
5 participants
Adding an option to display the cluster name for the question "Which alerts fire more than once per on-call shift (in the same cluster)?"
It will print the cluster name rather than the cluster ID