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

Fix ununsed param #189

Merged
merged 1 commit into from
Oct 26, 2023
Merged

Fix ununsed param #189

merged 1 commit into from
Oct 26, 2023

Conversation

mickmister
Copy link
Contributor

Summary

The scheduled CI job that runs every day has been failing for about a month due to this linting rule. @hanzei Any way we can be notified of this, maybe through a webhook to Mattermost?

@mickmister mickmister requested a review from hanzei October 24, 2023 15:16
@hanzei
Copy link
Contributor

hanzei commented Oct 24, 2023

A webhook makes sense. Actually, I would like to see such a notification for all plugin repos.

@mickmister
Copy link
Contributor Author

@hanzei Seems that we would get that for all plugin repos automatically by implementing this in the centralized plugin CI workflow. From this SO answer https://stackoverflow.com/a/63314229, it seems we can do something like this:

if: always() && (steps.ci/lint.outcome == 'failure')

We would want it to run if any of the steps fail though. Here's another suggested solution https://stackoverflow.com/a/74562058

if: ${{ always() && contains(needs.*.result, 'failure') }}

Given that we will trigger some step due to failure of a previous step, we would need a way to notify Mattermost. We could have a separate action that sends a message to Mattermost via webhook, accepting parameters like WEBHOOK_URL and MESSAGE. @toninis What do you think about this?

@toninis
Copy link

toninis commented Oct 25, 2023

@hanzei Seems that we would get that for all plugin repos automatically by implementing this in the centralized plugin CI workflow. From this SO answer https://stackoverflow.com/a/63314229, it seems we can do something like this:

if: always() && (steps.ci/lint.outcome == 'failure')

We would want it to run if any of the steps fail though. Here's another suggested solution https://stackoverflow.com/a/74562058

if: ${{ always() && contains(needs.*.result, 'failure') }}

Given that we will trigger some step due to failure of a previous step, we would need a way to notify Mattermost. We could have a separate action that sends a message to Mattermost via webhook, accepting parameters like WEBHOOK_URL and MESSAGE. @toninis What do you think about this?

The million dollar question is if we need the scheduled nightly builds in the first place .
We can enable centralised monitoring for our plugins here

I would suggest a new job with the below config. We avoid always() check here

if: failure() || cancelled()

Check on our delivery repository here for an example

@mickmister mickmister merged commit 5237d35 into master Oct 26, 2023
@mickmister mickmister deleted the fix-server-lint branch October 26, 2023 19:49
@mickmister
Copy link
Contributor Author

mickmister commented Oct 26, 2023

The million dollar question is if we need the scheduled nightly builds in the first place .
We can enable centralised monitoring for our plugins here

Check on our delivery repository here for an example

@toninis Thanks all makes sense to me. I'll take a look at implementing this.


The million dollar question is if we need the scheduled nightly builds in the first place

These jobs are configured per plugin repo, so if we want to stop them it should probably a centralized change.

There have been issues from these scheduled jobs running on forks, because of the resources they're using (mostly because of stored artifacts). There was a fix to turn off the scheduled job in this repo as a first step #180, though was closed in favor of a centralized fix mattermost/actions#10 for the piece related to space used storing artifacts.

Seeing that the scheduled job on this repo wasn't set up to have any indication of it failing, I'm thinking nobody has had any benefit from these jobs running on their forks either. I propose we centralize disallowing running in forks, possibly overridable through a parameter in case someone maintaining a plugin does want to. And if we want to continue running scheduled jobs for our org's repos, maybe we should change to one week instead.

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