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

Fix import of docker.py #1363

Closed
wants to merge 1 commit into from
Closed

Conversation

delanne
Copy link
Contributor

@delanne delanne commented Nov 17, 2014

with the current code, instead of import docker from docker-py, it imports docker from the buildslave directory:


(sandbox)xavierd@xavierd-ThinkStation-E31:~/Projects/myGitHub/buildbot-work/masterPrototype$ twistd -b -y buildbot.tac
> /home/xavierd/Projects/myGitHub/buildbot-work/sandbox/local/lib/python2.7/site-packages/twisted/internet/base.py(1191)run()
-> self.startRunning(installSignalHandlers=installSignalHandlers)
(Pdb) c
> /home/xavierd/Projects/myGitHub/buildbot-work/src/master/buildbot/buildslave/docker.py(30)<module>()
-> client = None
(Pdb) l
 25         from docker import client
 26         _hush_pyflakes = [client]
 27     except ImportError:
 28         import pdb
 29         pdb.set_trace()
 30  ->     client = None
 31     
 32     
 33     class DockerLatentBuildSlave(AbstractLatentBuildSlave):
 34         instance = None
 35     
(Pdb) import docker
(Pdb) from docker import client
*** ImportError: cannot import name client
(Pdb) p docker
<module 'buildbot.buildslave.docker' from '/home/xavierd/Projects/myGitHub/buildbot-work/src/master/buildbot/buildslave/docker.py'>
(Pdb) 

docker.py should be renamed. I choose to prefix the file with b (for buildbot).

Note: there's the same error for the libvirt support in buildbot.

@benallard
Copy link
Contributor

What does import sys; print sys.path says in that pdb environment ? And what does log.msg(sys.path) says ?

@sa2ajj
Copy link
Contributor

sa2ajj commented Nov 17, 2014

It's all cosmetics, but I'd prefer to have common prefix instead of just b. For example, ls_docker. This name will not be visible to the user (who makes use of plugins subsystem), and it will help immediately distinguish latent ones from non-latent.

@sa2ajj
Copy link
Contributor

sa2ajj commented Nov 17, 2014

Test case must refer to the new module name.

@benallard
Copy link
Contributor

Why not docker_latent ?

@sa2ajj
Copy link
Contributor

sa2ajj commented Nov 17, 2014

This suits me perfectly.

@benallard
Copy link
Contributor

A solution I'd rather use (and the reason libvirt is working) is to use absolute import ... the default in newer python ... I'll make a PR.

@delanne
Copy link
Contributor Author

delanne commented Nov 18, 2014

I close this PR, as #1364 was opened.

@delanne delanne closed this Nov 18, 2014
@delanne delanne deleted the DockerFixImport branch October 20, 2015 09:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants