Merge lp:~milo/lava-dispatcher/tar-repo into lp:lava-dispatcher
- tar-repo
- Merge into trunk
Status: | Merged |
---|---|
Merged at revision: | 640 |
Proposed branch: | lp:~milo/lava-dispatcher/tar-repo |
Merge into: | lp:lava-dispatcher |
Diff against target: |
102 lines (+55/-7) 1 file modified
lava_dispatcher/actions/lava_test_shell.py (+55/-7) |
To merge this branch: | bzr merge lp:~milo/lava-dispatcher/tar-repo |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Antonio Terceiro | Approve | ||
Review via email: mp+175274@code.launchpad.net |
Commit message
Description of the change
Added support to the dispatcher for tar-repo: a base64-encoded string of the content of a tar file.
This is necessary in order to support ongoing work on lava-tool (LAVA-15 card).
- 639. By Milo Casagrande
-
Fixed problem with extract directory and tar file.
- 640. By Milo Casagrande
-
Fixed typo.
Milo Casagrande (milo) wrote : | # |
- 641. By Milo Casagrande
-
Use 0 instead of nil for tar revision.
- 642. By Milo Casagrande
-
Merged from trunk, fixed conflicts.
- 643. By Milo Casagrande
-
Fixed PEP8 problem from previous merge.
Antonio Terceiro (terceiro) wrote : | # |
Hi Milo,
This looks nice. I have a few comments to be addressed.
review needs-fixing
> === modified file 'lava_dispatche
> --- lava_dispatcher
> +++ lava_dispatcher
> @@ -107,13 +107,16 @@
>
> from datetime import datetime
> from glob import glob
> +import base64
> import logging
> import os
> import pexpect
> import pkg_resources
> import shutil
> import stat
> +import StringIO
> import subprocess
> +import tarfile
> import tempfile
> import time
> from uuid import uuid4
> @@ -187,6 +190,37 @@
> logging.
>
>
> +def _get_testdef_
> + """Extracts the provided encoded tar archive into tmpdir."""
> + tardir = os.path.
> + temp_tar = os.path.
> +
> + try:
> + if not os.path.
> + logging.
> + os.makedirs(tardir)
> +
> + encoded_in = StringIO.
> + decoded_out = StringIO.StringIO()
> + base64.
> +
> + # The following two operations can also be done in memory
> + # using cStringIO.
> + # At the moment the tar file sent is not big, but that can change.
> + with open(temp_tar, "w") as write_tar:
> + write_tar.
> +
> + with tarfile.
> + tar.extractall(
> + except Exception as ex:
can we capture a more specific type of exception here? otherwise we may be
masking random exceptions that might happen above.
> + logging.
> + finally:
> + # Remove the temporary created tar file after it has been extracted.
> + if os.path.
> + os.unlink(temp_tar)
> + return tardir
> +
> +
> def _get_testdef_
> metadata = {'os': '', 'devices': '', 'environment': ''}
> metadata[
> @@ -255,8 +289,20 @@
> name = testdef_
> info = _bzr_info(
>
> + if 'tar-repo' in testdef_repo:
> + repo = _get_testdef_
> + # Default info structure, since we need something, but we have
> + # a tar file in this case.
> + info = {
> + "project_name": "Tar archived repository",
> + "branch_vcs": "tar",
> + "branch_revision": "0",
> + "branch_url": repo
> + }
> +
> if not repo or not info:
> logging.
> +
> test = testdef_
> with open(os.
> logging....
- 644. By Milo Casagrande
-
Added more specific exceptions.
Milo Casagrande (milo) wrote : | # |
Hi Antonio,
thanks for looking into it.
On Mon, Jul 22, 2013 at 11:46 PM, Antonio Terceiro
<email address hidden> wrote:
>
>> +def _get_testdef_
>> + """Extracts the provided encoded tar archive into tmpdir."""
>> + tardir = os.path.
>> + temp_tar = os.path.
>> +
>> + try:
>> + if not os.path.
>> + logging.
>> + os.makedirs(tardir)
>> +
>> + encoded_in = StringIO.
>> + decoded_out = StringIO.StringIO()
>> + base64.
>> +
>> + # The following two operations can also be done in memory
>> + # using cStringIO.
>> + # At the moment the tar file sent is not big, but that can change.
>> + with open(temp_tar, "w") as write_tar:
>> + write_tar.
>> +
>> + with tarfile.
>> + tar.extractall(
>> + except Exception as ex:
>
> can we capture a more specific type of exception here? otherwise we may be
> masking random exceptions that might happen above.
Fixed now, I used: (OSError, tarfile.TarError) to catch possible
errors with both open() and tarfile.open().
>> {'git-repo': {'type': 'string',
>> - 'optional': True},
>> - 'bzr-repo': {'type': 'string',
>> - 'optional': True},
>> - 'revision': {'type': 'string',
>> - 'optional': True},
>> - 'testdef': {'type': 'string',
>> - 'optional': True}
>> + 'optional': True},
>> + 'bzr-repo': {'type': 'string',
>> + 'optional': True},
>> + 'tar-repo': {'type': 'string',
>> + 'optional': True},
>> + 'revision': {'type': 'string',
>> + 'optional': True},
>> + 'testdef': {'type': 'string',
>> + 'optional': True}
>
> I think this indentation here got changed with a recent PEP8-related merge so
> that this will not merge cleanly, please check.
I merged from trunk and cleaned up, it should be OK.
Ciao.
--
Milo Casagrande | Automation Engineer
Linaro.org <www.linaro.org> │ Open source software for ARM SoCs
Antonio Terceiro (terceiro) wrote : | # |
On Tue, Jul 23, 2013 at 08:31:32AM -0000, Milo Casagrande wrote:
> >> + with tarfile.
> >> + tar.extractall(
> >> + except Exception as ex:
> >
> > can we capture a more specific type of exception here? otherwise we may be
> > masking random exceptions that might happen above.
>
> Fixed now, I used: (OSError, tarfile.TarError) to catch possible
> errors with both open() and tarfile.open().
cool
review approve
Preview Diff
1 | === modified file 'lava_dispatcher/actions/lava_test_shell.py' | |||
2 | --- lava_dispatcher/actions/lava_test_shell.py 2013-07-17 09:16:46 +0000 | |||
3 | +++ lava_dispatcher/actions/lava_test_shell.py 2013-07-23 08:14:26 +0000 | |||
4 | @@ -107,13 +107,16 @@ | |||
5 | 107 | 107 | ||
6 | 108 | from datetime import datetime | 108 | from datetime import datetime |
7 | 109 | from glob import glob | 109 | from glob import glob |
8 | 110 | import base64 | ||
9 | 110 | import logging | 111 | import logging |
10 | 111 | import os | 112 | import os |
11 | 112 | import pexpect | 113 | import pexpect |
12 | 113 | import pkg_resources | 114 | import pkg_resources |
13 | 114 | import shutil | 115 | import shutil |
14 | 115 | import stat | 116 | import stat |
15 | 117 | import StringIO | ||
16 | 116 | import subprocess | 118 | import subprocess |
17 | 119 | import tarfile | ||
18 | 117 | import tempfile | 120 | import tempfile |
19 | 118 | import time | 121 | import time |
20 | 119 | from uuid import uuid4 | 122 | from uuid import uuid4 |
21 | @@ -187,6 +190,37 @@ | |||
22 | 187 | logging.error('Unable to get test definition from bzr\n' + str(e)) | 190 | logging.error('Unable to get test definition from bzr\n' + str(e)) |
23 | 188 | 191 | ||
24 | 189 | 192 | ||
25 | 193 | def _get_testdef_tar_repo(testdef_repo, tmpdir): | ||
26 | 194 | """Extracts the provided encoded tar archive into tmpdir.""" | ||
27 | 195 | tardir = os.path.join(tmpdir, 'tartestrepo') | ||
28 | 196 | temp_tar = os.path.join(tmpdir, "tar-repo.tar") | ||
29 | 197 | |||
30 | 198 | try: | ||
31 | 199 | if not os.path.isdir(tardir): | ||
32 | 200 | logging.info("Creating directory to extract the tar archive into.") | ||
33 | 201 | os.makedirs(tardir) | ||
34 | 202 | |||
35 | 203 | encoded_in = StringIO.StringIO(testdef_repo) | ||
36 | 204 | decoded_out = StringIO.StringIO() | ||
37 | 205 | base64.decode(encoded_in, decoded_out) | ||
38 | 206 | |||
39 | 207 | # The following two operations can also be done in memory | ||
40 | 208 | # using cStringIO. | ||
41 | 209 | # At the moment the tar file sent is not big, but that can change. | ||
42 | 210 | with open(temp_tar, "w") as write_tar: | ||
43 | 211 | write_tar.write(decoded_out.getvalue()) | ||
44 | 212 | |||
45 | 213 | with tarfile.open(temp_tar) as tar: | ||
46 | 214 | tar.extractall(path=tardir) | ||
47 | 215 | except (OSError, tarfile.TarError) as ex: | ||
48 | 216 | logging.error("Error extracting the tar archive.\n" + str(ex)) | ||
49 | 217 | finally: | ||
50 | 218 | # Remove the temporary created tar file after it has been extracted. | ||
51 | 219 | if os.path.isfile(temp_tar): | ||
52 | 220 | os.unlink(temp_tar) | ||
53 | 221 | return tardir | ||
54 | 222 | |||
55 | 223 | |||
56 | 190 | def _get_testdef_info(testdef): | 224 | def _get_testdef_info(testdef): |
57 | 191 | metadata = {'os': '', 'devices': '', 'environment': ''} | 225 | metadata = {'os': '', 'devices': '', 'environment': ''} |
58 | 192 | metadata['description'] = testdef['metadata'].get('description') | 226 | metadata['description'] = testdef['metadata'].get('description') |
59 | @@ -255,8 +289,20 @@ | |||
60 | 255 | name = testdef_repo['bzr-repo'].replace('lp:', '').split('/')[-1] | 289 | name = testdef_repo['bzr-repo'].replace('lp:', '').split('/')[-1] |
61 | 256 | info = _bzr_info(testdef_repo['bzr-repo'], repo, name) | 290 | info = _bzr_info(testdef_repo['bzr-repo'], repo, name) |
62 | 257 | 291 | ||
63 | 292 | if 'tar-repo' in testdef_repo: | ||
64 | 293 | repo = _get_testdef_tar_repo(testdef_repo['tar-repo'], tmpdir) | ||
65 | 294 | # Default info structure, since we need something, but we have | ||
66 | 295 | # a tar file in this case. | ||
67 | 296 | info = { | ||
68 | 297 | "project_name": "Tar archived repository", | ||
69 | 298 | "branch_vcs": "tar", | ||
70 | 299 | "branch_revision": "0", | ||
71 | 300 | "branch_url": repo | ||
72 | 301 | } | ||
73 | 302 | |||
74 | 258 | if not repo or not info: | 303 | if not repo or not info: |
75 | 259 | logging.debug("Unable to identify specified repository. %s" % testdef_repo) | 304 | logging.debug("Unable to identify specified repository. %s" % testdef_repo) |
76 | 305 | |||
77 | 260 | test = testdef_repo.get('testdef', 'lavatest.yaml') | 306 | test = testdef_repo.get('testdef', 'lavatest.yaml') |
78 | 261 | with open(os.path.join(repo, test), 'r') as f: | 307 | with open(os.path.join(repo, test), 'r') as f: |
79 | 262 | logging.info('loading test definition ...') | 308 | logging.info('loading test definition ...') |
80 | @@ -462,13 +508,15 @@ | |||
81 | 462 | 'items': {'type': 'object', | 508 | 'items': {'type': 'object', |
82 | 463 | 'properties': | 509 | 'properties': |
83 | 464 | {'git-repo': {'type': 'string', | 510 | {'git-repo': {'type': 'string', |
91 | 465 | 'optional': True}, | 511 | 'optional': True}, |
92 | 466 | 'bzr-repo': {'type': 'string', | 512 | 'bzr-repo': {'type': 'string', |
93 | 467 | 'optional': True}, | 513 | 'optional': True}, |
94 | 468 | 'revision': {'type': 'string', | 514 | 'tar-repo': {'type': 'string', |
95 | 469 | 'optional': True}, | 515 | 'optional': True}, |
96 | 470 | 'testdef': {'type': 'string', | 516 | 'revision': {'type': 'string', |
97 | 471 | 'optional': True} | 517 | 'optional': True}, |
98 | 518 | 'testdef': {'type': 'string', | ||
99 | 519 | 'optional': True} | ||
100 | 472 | }, | 520 | }, |
101 | 473 | 'additionalProperties': False}, | 521 | 'additionalProperties': False}, |
102 | 474 | 'optional': True | 522 | 'optional': True |
I pushed new changes to fix a couple of problems found while running jobs.