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

auth login in non-interactive env should not block #197

Merged
merged 1 commit into from
Dec 18, 2024

Conversation

pawelkuc
Copy link
Contributor

If you execute nctl auth login in a non-interactive environment (e.g. CI/CD) and forget to set --api-token, the browser should not try to open - you should see an error message instead.

@pawelkuc pawelkuc force-pushed the not-block-with-no-tty branch 2 times, most recently from c1b8813 to bc278b0 Compare December 16, 2024 17:29
Copy link
Contributor

@thirdeyenick thirdeyenick left a comment

Choose a reason for hiding this comment

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

I am not sure why we have to add this specific option to the context and then have to change so many function calls (just because of that one option). For example, why do we need that interactive env information in the format.getPrinter function? Maybe you could help me in understanding this?

auth/login.go Outdated Show resolved Hide resolved
@pawelkuc pawelkuc force-pushed the not-block-with-no-tty branch 2 times, most recently from 9bb6cf3 to e4e402b Compare December 17, 2024 15:52
auth/login.go Outdated Show resolved Hide resolved
…ould not block

If you execute `nctl auth login` in a non-interactive environment (e.g. CI/CD)
and forget to set `--api-token`, the browser should not try to open - you
should see an error message instead.
@pawelkuc pawelkuc force-pushed the not-block-with-no-tty branch from e4e402b to efa74be Compare December 18, 2024 07:01
@pawelkuc pawelkuc merged commit a5ab996 into main Dec 18, 2024
2 checks passed
@pawelkuc pawelkuc deleted the not-block-with-no-tty branch December 18, 2024 08:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants