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

[JENKINS-73713] Do not filter params for completed run #9681

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jlehtnie
Copy link
Contributor

@jlehtnie jlehtnie commented Sep 2, 2024

If the Run has already completed there is no need to filter the parameters. This avoids unnecessary warnings about skipped parameters when getting environment for runs that have already finished.

See JENKINS-73713.

Testing done

Tested manually and regression run.

Proposed changelog entries

  • Remove unnecessary warnings about skipped parameters

Proposed upgrade guidelines

N/A

If the Run has already completed there is no need to filter the
parameters. This avoids unnecessary warnings about skipped parameters
when getting environment for runs that have already finished.
@daniel-beck
Copy link
Member

daniel-beck commented Sep 3, 2024

Why is the behavior you're describing in Jira a bug? Wouldn't having a different environment for polling than the reference run be the bug?

The warning is annoying and can probably be downgraded to FINE or so. We just need these messages to be very visible when we first introduce them so admins can have an idea why behavior changed, but it's been several years now.

@jlehtnie
Copy link
Contributor Author

jlehtnie commented Sep 4, 2024

Why is the behavior you're describing in Jira a bug? Wouldn't having a different environment for polling than the reference run be the bug?

This is not about having different environment for polling. In my understanding the problem is that we combine the current environment from the Project into the historical Run. The warning will be raised for the historical (completed) run. In my understanding the environment for a historical run should be whatever it was at the time when the run was completed (i.e. no need to filter anything).

E.g. in the reproduction instruction:

  1. create a job that uses scm polling and enable polling every minute. trigger at least one run
  2. install description-setter plugin
  3. configure job to use description-setter
  4. now when the polling runs, we can see the warning in the log

The polling will raise the warning when reading the environment for the previous, completed run. Why should we warn the user about environment for a build that has already been completed?

@jlehtnie
Copy link
Contributor Author

jlehtnie commented Sep 4, 2024

P.S. I'm against downgrading to FINE. Users need to know when parameters get dropped and the warning is very useful in context of currently active Run but for historical Run it simply makes no sense

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.

2 participants