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

feat: Use org and url from tokens #1673

Merged
merged 12 commits into from Jul 11, 2023
Merged

feat: Use org and url from tokens #1673

merged 12 commits into from Jul 11, 2023

Conversation

loewenheim
Copy link
Contributor

@loewenheim loewenheim commented Jul 7, 2023

The flow works like this:

  1. In configure_args and Config::from_file, we read the token first. If it's an org token, we attempt to parse it and save the contained org and url. A malformed org token results in an error.
  2. When we read a url or org (in Config::from_file, Config::get_org, Config::set_base_url), we check if we already have them from the token. If we do and the two values disagree, that's an error.

@loewenheim loewenheim self-assigned this Jul 7, 2023
@loewenheim loewenheim linked an issue Jul 7, 2023 that may be closed by this pull request
@loewenheim loewenheim marked this pull request as ready for review July 10, 2023 11:16
Copy link
Member

@mydea mydea left a comment

Choose a reason for hiding this comment

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

nice! Can't comment too much on the rust code, but logic seems great to me 👍 LMK once this has been released somehow, then I can update the bundler plugins to use this as well 🚀

Copy link
Contributor

@kamilogorek kamilogorek left a comment

Choose a reason for hiding this comment

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

LGTM! Can we maybe get some integration tests around value mismatches?

@loewenheim
Copy link
Contributor Author

You mean trycmd tests? Yeah, that's probably a good idea.

@kamilogorek
Copy link
Contributor

Yup. It should be trivial to get them going with env vars

@loewenheim
Copy link
Contributor Author

loewenheim commented Jul 10, 2023

Don't even need env vars, unless you think I should explicitly try those as well.

@loewenheim loewenheim merged commit a99e1bf into master Jul 11, 2023
16 checks passed
@loewenheim loewenheim deleted the feat/org-tokens branch July 11, 2023 15:25
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.

Handle Org Auth Tokens
3 participants