Project

General

Profile

Actions

Bug #10587

closed

All Python CLI utilities should report --version

Added by Tom Morris over 7 years ago. Updated about 7 years ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Category:
-
Target version:
Start date:
11/22/2016
Due date:
% Done:

100%

Estimated time:
(Total: 0.00 h)
Story points:
1.0

Subtasks 1 (0 open1 closed)

Task #10624: Review 10587-python-cli-versionResolvedTom Clegg11/22/2016

Actions

Related issues

Related to Arvados - Feature #10666: All Arvados components should report their versionResolvedLucas Di Pentima12/05/2016

Actions
Actions #1

Updated by Tom Morris over 7 years ago

  • Subject changed from All CLI utilities should report --version to All Python CLI utilities should report --version
  • Assigned To set to Lucas Di Pentima
  • Story points set to 1.0
Actions #2

Updated by Lucas Di Pentima about 7 years ago

  • Status changed from New to In Progress
Actions #3

Updated by Lucas Di Pentima about 7 years ago

Test run at: https://ci.curoverse.com/job/developer-run-tests/87/
Updates: fee8873

Added --version to the following commands:

  • arv-ls
  • arv-put
  • arv-ws
  • arv-get
  • arv-copy
  • arv-run
  • arv-normalize
  • arv-keepdocker
  • arv-mount
  • arvados-node-manager

Also added tests to confirm the new argument.

Actions #4

Updated by Tom Clegg about 7 years ago

Looks like we need to add setuptools dependency to our deb/rpm packages, like we did for arvados-cwl-runner, in order to use this mechanism.

I guess it also makes sense to add setuptools to install_requires in setup.py.

Tried in wheezy, and found that the wheezy version of python-setuptools (0.6.24-1) does work, so at least we don't have to build another new package.

I'm very wary of using multiprocessing in tests, especially arv-put which definitely uses threading. Combining multiprocessing and threading in Python seems to be a recipe for disaster (or at least unbounded debugging efforts). Can we get what we need just by catching SystemExit? In source:services/fuse/tests/test_command_args.py it looks like this works for argparse. Failing that, maybe subprocess?

Actions #5

Updated by Lucas Di Pentima about 7 years ago

Updates at: ceae525
Test suite run at: https://ci.curoverse.com/job/developer-run-tests/88/

  • Changed those tests depending on multiprocessing to use a context manager stderr/stdout redirector and a SystemExit catching assertion as requested.
  • Added setuptools dependencies on package building script and setup.py install_require definitions.
Actions #7

Updated by Tom Clegg about 7 years ago

LGTM thanks!

Actions #8

Updated by Lucas Di Pentima about 7 years ago

  • Status changed from In Progress to Resolved
  • % Done changed from 0 to 100

Applied in changeset arvados|commit:0514b290f5ff9a2700b599bf6fb19a468a73c3fb.

Actions

Also available in: Atom PDF