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

boxcli: devbox cache enable #2157

Merged
merged 1 commit into from
Jun 20, 2024
Merged

boxcli: devbox cache enable #2157

merged 1 commit into from
Jun 20, 2024

Conversation

gcurtis
Copy link
Collaborator

@gcurtis gcurtis commented Jun 17, 2024

Don't merge until the new cloud.jetify.com/team/cache URL is live.


Add a devbox cache enable subcommand that:

  1. Opens a browser and performs the login flow is the user isn't logged in.
  2. Configures Nix to use the Jetify cache if it isn't already.

The login flow redirects to the new cloud.jetify.com/team/cache URL to let the user know the cache is enabled.

Add a `devbox cache enable` subcommand that:

1. Opens a browser and performs the login flow is the user isn't logged
   in.
2. Configures Nix to use the Jetify cache if it isn't already.

The login flow redirects to the new https://cloud.jetify.com/team/cache
URL to let the user know the cache is enabled.
@gcurtis gcurtis requested a review from mikeland73 June 17, 2024 21:39
if err != nil {
return err
}
sess, _ := auth.GetSessions()
Copy link
Contributor

Choose a reason for hiding this comment

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

You probably want GetSession?

GetSessions returned unrefreshed sessions. It's more useful for trying to figure out which session to use

Copy link
Contributor

Choose a reason for hiding this comment

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

sess, err := auth.GetSession()
needLogin := errors.Is(err, auth.ErrNotLoggedIn)

Copy link
Contributor

Choose a reason for hiding this comment

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

There's also identity.GenSession which also uses the API token, but not sure that makes sense for this command?

Long term I think having an API token should be a drop in replacement for logging in.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think we just want to know if the user has logged in before. We don't necessarily need a refreshed token.

I had the same question about whether or not to use the API token. I decided to ignore it, which is why I didn't use identity.Peek. But now I'm having second thoughts and wondering if we should just remove devbox cache configure and make devbox cache enable do everything. Is there ever a situation where it makes sense to configure the cache without having an account or API token?

What if devbox cache enable did the following:

  1. If there's a DEVBOX_API_TOKEN, skip to step 3.
  2. If the user has never logged in before (no sessions), go through the login flow.
  3. Configure Nix.


// AuthRedirectCache redirects to the "Cache" tab in the dashboard for
// the authenticated organization.
AuthRedirectCache = path.Join(build.DashboardHostname(), "team", "cache")
Copy link
Contributor

Choose a reason for hiding this comment

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

See comment about url in other PR.

@gcurtis gcurtis merged commit ac28b01 into main Jun 20, 2024
24 checks passed
@gcurtis gcurtis deleted the gcurtis/cache-enable branch June 20, 2024 16:10
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.

None yet

2 participants