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 securedrop-admin crash when no operation given under Python 3 #4912

Merged
merged 1 commit into from
Oct 16, 2019

Conversation

rmol
Copy link
Contributor

@rmol rmol commented Oct 16, 2019

Status

Ready for review

Description of Changes

towards #4910.

Under Python 3, running securedrop-admin with no positional argument results in an ugly error, due to https://bugs.python.org/issue16308.

Under Python 3.7, we could simply add dest and required arguments to the add_subparsers call, but required isn't available in Python 3.5, so would break under Tails 3.

Testing

Run securedrop/bin/dev-shell bash to start a shell in the dev container, then run:

  • cd ..
  • ./securedrop-admin setup
  • ./securedrop-admin -v

You should see Please specify an operation. followed by the securedrop-admin help.

Deployment

This only affects the admin workstation.

Checklist

If you made changes to securedrop-admin:

  • Linting and tests (make -C admin test) pass in the admin development container

If you made non-trivial code changes:

  • I have written a test plan and validated it for this PR

Under Python 3, running securedrop-admin with no positional argument
results in an ugly error, due to https://bugs.python.org/issue16308.

Under Python 3.7, we could simply add dest and required arguments to
the add_subparsers call, but required isn't available in Python 3.5,
so would break under Tails 3.
Copy link
Contributor

@kushaldas kushaldas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is good. Approved.

@emkll
Copy link
Contributor

emkll commented Oct 16, 2019

Thanks @rmol went through the fix and does what's advertised, ./securedrop-admin now works and prints the help page, as expected.

I've also edited the PR description to say that it doesn't completely resolve #4910 : while it allows a user to run ./securedrop-admin with no argument, it does not address the issue that the venv in admin/.ven3 needs to be destroyed/rebuilt for Tails 4. We should, as part of #4910 either

  1. Provide docs for the 1.0.0 -> 1.1.0 release to indicate that the venv needs to be destroyed and rebuilt
  2. Automate the process (this might be difficult as we cannot do this as part of the updater code, we would need to either create a new ./securedrop-admin option or modify ./securedrop-admin setup (check if tails3 and python version in venv is 3.5)

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

Successfully merging this pull request may close these issues.

3 participants