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

Giving a Property as workersrcs to MultipleFileUpload breaks #3167

Closed
benallard opened this issue May 2, 2017 · 6 comments · Fixed by #3172
Closed

Giving a Property as workersrcs to MultipleFileUpload breaks #3167

benallard opened this issue May 2, 2017 · 6 comments · Fixed by #3172

Comments

@benallard
Copy link
Contributor

This issue was introduced by 4173dac.

I give a Property parameter to MultipleFileUploads, the property resolves to a list.

Due to the mentioned commit, The value is now a list of list, and this breaks.

2017-05-02 12:55:50+0200 [-] MultipleFileUpload started, from worker [[u'build/filename']] to master '/home/buildsystem/master/public_html/destination'
2017-05-02 12:55:50+0200 [-] BuildStep.failed; traceback follows
        Traceback (most recent call last):
          File "/home/buildsystem/bb-master/local/lib/python2.7/site-packages/twisted/internet/defer.py", line 1357, in gotResult
            _inlineCallbacks(r, g, deferred)
          File "/home/buildsystem/bb-master/local/lib/python2.7/site-packages/twisted/internet/defer.py", line 1301, in _inlineCallbacks
            result = g.send(result)
          File "/home/buildsystem/bb-master/local/lib/python2.7/site-packages/buildbot/process/buildstep.py", line 547, in startStep
            self.results = yield self.run()
          File "/home/buildsystem/bb-master/local/lib/python2.7/site-packages/twisted/internet/defer.py", line 1447, in unwindGenerator
            return _inlineCallbacks(None, gen, Deferred())
        --- <exception caught here> ---
          File "/home/buildsystem/bb-master/local/lib/python2.7/site-packages/twisted/internet/defer.py", line 1301, in _inlineCallbacks
            result = g.send(result)
          File "/home/buildsystem/bb-master/local/lib/python2.7/site-packages/buildbot/process/buildstep.py", line 677, in run
            results = yield self._start_deferred
        exceptions.AttributeError: 'list' object has no attribute 'rfind'
@benallard
Copy link
Contributor Author

@noc0lour

@benallard
Copy link
Contributor Author

I'd be in favor of reverting the said commit. Why should MultipleFileUploads be able to magically behave as if it weren't Multiple ?

@tardyp
Copy link
Member

tardyp commented May 3, 2017

I think we can easily add
and not IRenderable.providedBy(workersrc))
like in https://github.com/buildbot/buildbot/blob/master/master/buildbot/steps/cmake.py#L52

The commit is part of the more general and useful PR:

transfer: add glob parameter to MultipleFileUpload #2635

@benallard
Copy link
Contributor Author

If we do that the given renderable will have to render as a list. Bypassing the magical part (i.e. a property that render as a string will not work). IMHO, glob support (granted, useful) doesn't require the type of the parameter to be a string. Additionally, it's not even documented that a string can be given as parameter. Lastly, even the tests don't exerce the combination glob + string.

@tardyp
Copy link
Member

tardyp commented May 3, 2017

@benallard make sense. @noc0lour please defend your design :)

@noc0lour
Copy link
Contributor

noc0lour commented May 3, 2017

I don't see why MultipleFileUpload shouldn't be able to upload just one file. e.g. your renderable returns only one item or None.
Probably this check is better placed start().
Can you add the test? I'll submit a PR with a proposed fix which should resolve your issue.

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

Successfully merging a pull request may close this issue.

4 participants