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

Implement "Create Silence" Feature as a Modal #523

Merged
merged 1 commit into from
Jul 2, 2024

Conversation

bai1024
Copy link
Contributor

@bai1024 bai1024 commented Jun 19, 2024

  1. Modal Implementation : Change the current setting interface to a modal design.
  2. Modal Features : Within the modal, Provide input fields to set Silence
  3. Support Flexible Silence Setting : Add silence setting field(Matcher), and target field. Matcher field is working like the behavior of Grafana's one
image

@bai1024 bai1024 requested a review from a team as a code owner June 19, 2024 08:56
Copy link
Contributor

@vincent-olivert-riera vincent-olivert-riera left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First review spin done. Nice job :)
Please have a look at my code suggestions and comments.
Thank you very much.

promgen/templates/promgen/vue/silence_modal.html Outdated Show resolved Hide resolved
promgen/templates/promgen/vue/silence_modal.html Outdated Show resolved Hide resolved
promgen/static/css/promgen.css Show resolved Hide resolved
promgen/static/js/promgen.vue.js Outdated Show resolved Hide resolved
promgen/templates/promgen/service_block.html Outdated Show resolved Hide resolved
promgen/static/js/promgen.vue.js Outdated Show resolved Hide resolved
promgen/static/js/promgen.vue.js Outdated Show resolved Hide resolved
Copy link
Contributor

@vincent-olivert-riera vincent-olivert-riera left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have updated everything according to the first review cycle and squashed the commits. Please check.

Second review spin finished.

In some parts I could do GitHub's code suggestion that are easier to commit. In other parts that was not possible due to GitHub limitations.

Please check.

promgen/static/js/promgen.vue.js Outdated Show resolved Hide resolved
promgen/static/js/promgen.vue.js Outdated Show resolved Hide resolved
promgen/static/js/promgen.vue.js Outdated Show resolved Hide resolved
promgen/static/js/promgen.vue.js Outdated Show resolved Hide resolved
promgen/static/js/promgen.vue.js Outdated Show resolved Hide resolved
promgen/static/js/promgen.vue.js Outdated Show resolved Hide resolved
promgen/static/js/promgen.vue.js Show resolved Hide resolved
promgen/static/js/promgen.vue.js Show resolved Hide resolved
promgen/templates/promgen/vue/silence_modal.html Outdated Show resolved Hide resolved
@bai1024 bai1024 force-pushed the create-slience-modal branch 4 times, most recently from af63811 to 0cd465c Compare July 2, 2024 08:32
Copy link
Contributor

@vincent-olivert-riera vincent-olivert-riera left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@vincent-olivert-riera vincent-olivert-riera merged commit 9eeb79e into line:master Jul 2, 2024
5 checks passed
<ul class="list-inline promgen-labels-list">
<li v-for="(value, label) in state.labels">
<div class="promgen-label-target">
<span>[[label]]=~[[value]]</span>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is somewhat incorrect. We are not always using =~ matches for everything. I think only the instance field is a =~ while others should generally be a regular = match 🤔

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A little buried but

kwargs["matchers"] = [
{
"name": name,
"value": value,
"isEqual": True, # Right now we only support =~
"isRegex": True if value.endswith("*") else False,
}
for name, value in labels.items()

The only time it is a =~ is when our matcher ends with * and in Promgen that is generally only for our quick silence for instance/host

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants