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

Setting timeout=None to the CopyDirectory step has no effect. #3032

Closed
tardyp opened this issue Mar 10, 2017 · 4 comments
Closed

Setting timeout=None to the CopyDirectory step has no effect. #3032

tardyp opened this issue Mar 10, 2017 · 4 comments

Comments

@tardyp
Copy link
Member

tardyp commented Mar 10, 2017

This ticket is a migrated Trac ticket 3669

People contributed to the original ticket: benoit.allard@...
Ticket created on: Feb 28 2017
Ticket last modified on: Feb 28 2017


According to the documentation, this should disable the timeout altogether.

Unfortunately, the timeout value is given to the worker only if the value evaluates to 'True-ish'. The worker on its side, when not given a value uses 120sec. Making the timeout=None useless.


Comment from: @ben
Date: Feb 28 2017

The workaround is to use a bigger value, like 1200.

@leopoldwe
Copy link
Contributor

Is this issue still open? I'd like to work on it.

@tardyp
Copy link
Member Author

tardyp commented Jul 1, 2020

hi @leopoldwe there is an old PR that was seems to have a wrong fix at #3855.
this was never finished. Please let us know your first investigations

@leopoldwe
Copy link
Contributor

I think the problem exists in buildbot/master/steps/worker.py, where on line 124 we have:
def __init__(self, src, dest, timeout=None, maxTime=None, **kwargs):
where the timeout defaults to None. Later, in the start function (lines 135 and 136), the truthiness of timeout is evaluated:
if self.timeout:
args['timeout'] = self.timeout

Later, the get statement in fs.py that is modified by the old pull statement finds args['timeout'] empty, and sets timeout to 120.

The solution I can think of is to set the default to 120 (to be in line with documentation) and remove the if statement. The tests would also need to be updated, as they expect timeout to be None if no argument is passed.

@tardyp
Copy link
Member Author

tardyp commented Jul 5, 2020

@leopoldwe I agree on your root-cause finding. removing the if self.timeout will fix the issue. Can you please submit a PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants