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

[PY-597] Darwin-py extra authenticate options #742

Merged
merged 4 commits into from Dec 12, 2023
Merged

[PY-597] Darwin-py extra authenticate options #742

merged 4 commits into from Dec 12, 2023

Conversation

Nathanjp91
Copy link
Contributor

@Nathanjp91 Nathanjp91 commented Dec 8, 2023

Problem

Darwin Authentication currently has no way to programmatically set authenticate options without engaging the CLI

Solution

Set extra options, allowing to pass in api-key, default-team and dataset directory as CLI flags, as well as setting env variables
.env

DARWIN_API_KEY=' '
DARWIN_TEAM=' '
DARWIN_DATASETS_DIR=' '

or via CLI commands
darwin authenticate --api_key {key} --default_team {team} --datasets_dir {dir}

Changelog

darwin-py can now be authenticated via env variables and CLI flags without requiring input

Copy link

linear bot commented Dec 8, 2023

Copy link
Contributor

@JBWilkie JBWilkie left a comment

Choose a reason for hiding this comment

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

Tested all options, all working on my end, LGTM

Just one small point on message output correctness

darwin/cli.py Outdated
api_key = os.getenv("DARWIN_API_KEY")
api_key = os.getenv("DARWIN_API_KEY") or args.api_key
default_team = os.getenv("DARWIN_TEAM") or args.default_team
datasets_dir = os.getenv("DARWIN_DATASETS_DIR") or args.datasets_dir
if api_key:
print("Using API key from DARWIN_API_KEY")
Copy link
Contributor

Choose a reason for hiding this comment

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

When I pass an API key with --api_key, I see Using API key from DARWIN_API_KEY

Is this technically incorrect, since it's actually from a CLI arg?

Copy link
Contributor

Choose a reason for hiding this comment

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

Same goes for the --default_team and --dataset_dir flags

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated for that

@Nathanjp91 Nathanjp91 merged commit ec59846 into master Dec 12, 2023
13 checks passed
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.

None yet

2 participants