ros-infrastructure / ros_buildfarm

ROS buildfarm based on Docker
Apache License 2.0
77 stars 95 forks source link

Avoid jenkinsapi trying to fetch all jobs. #1045

Closed nuclearsandwich closed 2 months ago

nuclearsandwich commented 2 months ago

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.