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

Disable DiskUsageCollector when its option is unchecked (JENKINS-66790) #364

Merged
merged 1 commit into from Mar 7, 2022

Conversation

agabrys
Copy link

@agabrys agabrys commented Feb 25, 2022

Fixes JENKINS-66790.

Changes proposed

The DiskUsageCollector class uses the ConfigurationUtils#getCollectDiskUsage method to determine whether the disk usage statistics should be collected or not. Unfortunately, the mentioned method ignores the checkbox configuration option available on the Configure System page. The PrometheusConfiguration class keeps the requested value in the collectDiskUsage field which is set with the default value if needed. The ConfigurationUtils#getCollectDiskUsage method is removed and the PrometheusConfiguration#getCollectDiskUsage method is used instead.

The PrometheusConfiguration#getDefaultCollectDiskUsage method is no longer used, so it has been removed too.

Checklist

  • Includes tests covering the new functionality?
  • Ready for review
  • Follows CONTRIBUTING rules

Notify

@markyjackson-taulia

@agabrys
Copy link
Author

agabrys commented Feb 25, 2022

I'll be able to test the changes right after I get information how to build the project without any issues (read more: #337 (comment)).

@agabrys agabrys force-pushed the feature/JENKINS-66790 branch 3 times, most recently from 7027c85 to 02788d8 Compare February 27, 2022 14:50
The `DiskUsageCollector` class uses the `ConfigurationUtils#getCollectDiskUsage` method to determine whether the disk usage statistics should be collected or not. Unfortunately, the mentioned method ignores the checkbox configuration option available on the `Configure System` page. The `PrometheusConfiguration` class keeps the requested value in the `collectDiskUsage` field which is set with the default value if needed. The `ConfigurationUtils#getCollectDiskUsage` method is removed and the `PrometheusConfiguration#getCollectDiskUsage` method is used instead.

The `PrometheusConfiguration#getDefaultCollectDiskUsage` method is no longer used, so it has been removed too.
@agabrys agabrys marked this pull request as ready for review February 27, 2022 16:50
@agabrys
Copy link
Author

agabrys commented Feb 27, 2022

@markyjackson-taulia please, take a look. If you agree with changes, it would be great to release a new version of the plugin after the merge.

@markjacksonfishing
Copy link

I agree with the change and will do a release tomorrow night

@agabrys
Copy link
Author

agabrys commented Mar 6, 2022

@markyjackson-taulia could you provide ETA when it should be available?

@markjacksonfishing
Copy link

My hope is tomorrow. I am currently on a business trip

@markjacksonfishing markjacksonfishing merged commit c5d4ce1 into jenkinsci:master Mar 7, 2022
@markjacksonfishing
Copy link

@agabrys Pr has been approved and merged. Release has started and it will take several hours for it to become available in the update center.

Thank you for this work and your patients.

@markjacksonfishing
Copy link

Running into test errors: https://pastebin.com/vQi71zNS

cc @agabrys

@agabrys agabrys deleted the feature/JENKINS-66790 branch March 7, 2022 20:29
@agabrys
Copy link
Author

agabrys commented Mar 7, 2022

The plugin should be built with Java 8, but Java 17 is used:

[...]
module java.base does not "opens java.util" to unnamed module @4d937939
[...]
[INFO] JDK Version: 17.0.2 normalized as: 17.0.2
[...]

@markjacksonfishing
Copy link

@agabrys thanks for catching that. I must have changed and forgot to put that back.
Plugin has been released, give it a few hours to show up in the update center.

 [INFO] ------------------------------------------------------------------------
    [INFO] BUILD SUCCESS
    [INFO] ------------------------------------------------------------------------
    [INFO] Total time:  32.353 s
    [INFO] Finished at: 2022-03-07T14:25:18-08:00
    [INFO] ------------------------------------------------------------------------
[INFO] Cleaning up after release...
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  43.319 s
[INFO] Finished at: 2022-03-07T14:25:18-08:00
[INFO] ------------------------------------------------------------------------

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.

None yet

2 participants