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

Token path argument #329

Merged
merged 7 commits into from
Mar 3, 2022
Merged

Token path argument #329

merged 7 commits into from
Mar 3, 2022

Conversation

alneberg
Copy link
Contributor

@alneberg alneberg commented Mar 2, 2022

A new argument --token-path is added and used as such:

dds --token-path my_custom_token auth login
dds --token-path my_custom_token ls

I also moved the username printing for the logo inside the dds_main. As a consequence, it will now not be printed if you run dds --help or dds --version, but otherwise it should be printed.

Also, if the username is not found, for example if no token is found, it's not printed at all. This can of course be adjusted however we want to.

Feel free to squash merge this as the commits became a bit messy.

Before submitting a PR to the dev branch:

  • Tests passing
  • Black formatting
  • Rebase/merge the dev branch
  • Note in the CHANGELOG

Additional checks before submitting a PR to the master branch:

  • Change version in setup.py (?)

@alneberg alneberg marked this pull request as draft March 2, 2022 15:46
@alneberg alneberg marked this pull request as ready for review March 2, 2022 20:58
@alneberg alneberg requested review from i-oden and aanil March 2, 2022 20:58
def __init__(self):
self.token_file = dds_cli.TOKEN_FILE
def __init__(self, token_path=None):
if token_path is None:
Copy link
Member

@i-oden i-oden Mar 3, 2022

Choose a reason for hiding this comment

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

(Minor comment: You don't need the is None here, if not token_path also works)

Copy link
Member

@i-oden i-oden left a comment

Choose a reason for hiding this comment

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

Nice! Looks good and works!

Copy link
Contributor

@aanil aanil left a comment

Choose a reason for hiding this comment

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

👍

@i-oden i-oden merged commit 51ebb71 into ScilifelabDataCentre:dev Mar 3, 2022
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