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

Avoid jenkinsapi trying to fetch all jobs. #1045

Merged
merged 3 commits into from
May 2, 2024

Conversation

nuclearsandwich
Copy link
Contributor

By default the jenkinsapi Jenkins class will trigger an eager loading of
all Jenkins jobs via the API. Our Jenkins server buckles under the
weight of this and so we do not want to do this until we need to and
ideally future changes will replace unfiltered polling completely.

There's a malinteraction with the jenkinsapi library and Python
convention which has been causing extremely large HTTP requests whenever
the truthiness of an instance of the Jenkins class is tested.

Jenkins defines a __len__ method which is used to determine
truthiness. But this method triggers a fetch of every job on our Jenkins
host which can take 2 - 5 minutes to return due to the sheer volume of
jobs.

By avoiding this in favor of an explicit check whether jenkins is an
instance of the JenkinsProxy class we can save taking a really big
runtime hit and cut down on spurious failures due to timeouts.

This is an alternative to #1044 which changes what we're testing since it turns out that there are codepaths in ros_buildfarm that use tri-state logic JenkinsProxy|None|False on purpose to distinguish between intentional absence of the Jenkins object and an uninitialized Jenkins object.

I'd like to factor away from that or at least make it louder through the use of an explicit class for DontUseJenkins rather than knowing that False and None should be treated differently but this alternative should meet the original needs of #1044 without creating logic errors downstream.

By default the jenkinsapi Jenkins class will trigger an eager loading of
all Jenkins jobs via the API. Our Jenkins server buckles under the
weight of this and so we do not want to do this until we need to and
ideally future changes will replace unfiltered polling completely.
@nuclearsandwich nuclearsandwich requested a review from cottsay April 27, 2024 00:33
@nuclearsandwich nuclearsandwich self-assigned this Apr 27, 2024
There's a malinteraction with the jenkinsapi library and Python
convention which has been causing extremely large HTTP requests whenever
the truthiness of an instance of the Jenkins class is tested.

Jenkins defines a `__len__` method which is used to determine
truthiness. But this method triggers a fetch of every job on our Jenkins
host which can take 2 - 5 minutes to return due to the sheer volume of
jobs.

By avoiding this in favor of an explicit check whether jenkins is an
instance of the JenkinsProxy class we can save taking a really big
runtime hit and cut down on spurious failures due to timeouts.
@nuclearsandwich nuclearsandwich force-pushed the revenge-of-jenkins-poll branch from 60131b7 to 5ecbe8f Compare April 27, 2024 04:48
Copy link
Member

@cottsay cottsay left a comment

Choose a reason for hiding this comment

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

+1 for this more verbose but safer approach

@nuclearsandwich nuclearsandwich merged commit 7fa41d2 into master May 2, 2024
32 checks passed
@nuclearsandwich nuclearsandwich deleted the revenge-of-jenkins-poll branch May 2, 2024 15:30
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