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

Users cannot notice statuses errors in notification settings. #2051

Closed
ibu1224 opened this issue Apr 2, 2020 · 7 comments
Closed

Users cannot notice statuses errors in notification settings. #2051

ibu1224 opened this issue Apr 2, 2020 · 7 comments
Labels

Comments

@ibu1224
Copy link
Contributor

ibu1224 commented Apr 2, 2020

What happened:
Currently, I can not notice the setting mistake of the notification of screwdriver.yaml.

What you expected to happen:
Users need to be aware of any mis-configuration in some way.
How to reproduce it:
Wrong settings, screwdriver.yaml
e.g.

  shared:
    image: node:10
    settings:
      slack:
        channels:
          - ibu1224-test-channel
        statuses:
          - SUCCCESS <---  typo
          - FAILURE
@ibu1224
Copy link
Contributor Author

ibu1224 commented Apr 2, 2020

I will solve this problem.
I'm thinking of an idea so please wait a moment.

@ibu1224 ibu1224 changed the title Users can notice errors in notification settings. Users can notice statuses errors in notification settings. Apr 2, 2020
@ibu1224
Copy link
Contributor Author

ibu1224 commented Apr 2, 2020

Here are idea to solve this problem.
Each notification repository has a validation by joi.
Therefore, add a new function to validate notifications in config-parser.
Each repository has a STATUSES_MAP exists, Use that Key (e.g. SUCCESS) as validation.

@jithine, @kumada626, and contributors.
With agreement, I will start implementing.I would appreciate your feedback.

@tk3fftk
Copy link
Member

tk3fftk commented Apr 3, 2020

There is a kind of same issue in annotations.

@wahapo
Copy link
Contributor

wahapo commented Apr 6, 2020

It reminds me of my checkAdditionalRules function fix. LGTM.

@jithine jithine changed the title Users can notice statuses errors in notification settings. Users cannot notice statuses errors in notification settings. Apr 8, 2020
@jithine jithine added the bug label Apr 8, 2020
@jithine
Copy link
Member

jithine commented Apr 8, 2020

@ibu1224 your solution sounds good to me.

@jithine
Copy link
Member

jithine commented Jan 7, 2021

@ibu1224 is there way to make existing pipelines not break and still have validator throw errors and prevent new pipeline creation ?
So existing pipelines always works and only new pipelines are broken ?

@ibu1224
Copy link
Contributor Author

ibu1224 commented Jan 7, 2021

@jithine
Hmm ... I think it's difficult.
Notification settings are added at the user's discretion.

Therefore.
We need to validate all pipeline triggers to be able to notice mistakes in the notification settings.

Otherwise, users will not be able to notice that the screwdriver.yaml is wrong.
The pipeline will throw a config-parse-error if the screwdriver.yaml is wrong, whether new or existing.

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

No branches or pull requests

4 participants