Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
Fix upload_scripts and INFRA failures #166
Conversation
|
This should fix both the new INFRA message failure as well as the underlying cause. |
| - self.info['unit_name'], dest) | ||
| - subprocess.check_call(cmd.split()) | ||
| + src = os.path.join(source, f) | ||
| + if os.path.isfile(src): |
tvansteenburgh
Nov 9, 2016
Member
Why would src not be a file? Perhaps the glob pattern just needs to be changed?
johnsca
Nov 9, 2016
Member
See the stack trace in the linked issue. It's picking up __pycache__ directories. Changing the glob is a valid alternative.
tvansteenburgh
Nov 9, 2016
Member
Ah, okay. Maybe a comment to that effect? Otherwise, if you're just looking at the source tree and this code, it's not clear why this condition is needed.
kwmonroe
Nov 9, 2016
Member
If there are valid directories in /tmp/amulet, scp -r would be another option. The comment notes that this doesn't work, but it does (at least with juju2). Syntax is like this:
juju scp -- -r {} {}:{}...
If the only dir in /tmp/amulet is __pycache__, then sure, let's glob and only scp files.
johnsca
Nov 9, 2016
Member
There are four files that this needs to pick up: https://github.com/juju/amulet/tree/master/amulet/unit-scripts/amulet
It looks like this is a side-effect of 7adbf808 where you relaxed the glob specifically to pick up the .sh file, @tvansteenburgh.
@kwmonroe We could use -r but we don't actually want to pick up the __pycache__ (or any other) directories.
I think this is the cleanest solution. We could change the glob to *.[ps][yh] but that seems a bit ugly.
|
@johnsca nice job with Path().files() -- added bonus will be less juju scp logs in the cwr console output. LGTM, but giving @tvansteenburgh a chance before merging. |
johnsca commentedNov 9, 2016
•
Edited 1 time
-
johnsca
Nov 9, 2016
Fixes #165