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: base quickstart on remote existing target #999

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

adaam2
Copy link
Contributor

@adaam2 adaam2 commented Oct 7, 2024

This PR adds the ability to bootstrap a new target using a pre-existing specification hosted on Speakeasy's registry. The CLI will now show the most recent 5 targets that have been interacted with (based on the CLI event stream).

If there are any recent target candidates in the workspace, then the user will be presented with the option to choose from an existing SDK or create from scratch at the beginning of Quickstart. If there are no previous generations in a workspace, then the existing "new user" flow will be shown where the user can enter a filepath to a spec:

CleanShot 2024-10-07 at 13 46 20@2x

Then selecting from the list of recent SDKs:

CleanShot 2024-10-07 at 13 46 32@2x

(n.b the look and feel of the select list in the 2nd screenshot will change / look much better once the corresponding charmbracelet/huh#424 pr has been merged)

The end result is that a new workflow file is created that references the existing Registry URI for the spec from the previous target

Next steps

  • Try and re-use as much of the prior target's gen.yaml

@adaam2 adaam2 force-pushed the feat/quickstart-existing-spec branch from 2276055 to 303e234 Compare October 7, 2024 12:50
@adaam2 adaam2 force-pushed the feat/quickstart-existing-spec branch from 303e234 to 4a0e465 Compare October 7, 2024 12:53
Copy link
Member

@simplesagar simplesagar left a comment

Choose a reason for hiding this comment

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

+1 for the concept

return "", fmt.Errorf("could not generate registry uri: missing organization or workspace slug")
}

return fmt.Sprintf("registry.speakeasyapi.dev/%s/%s/%s@%s", orgSlug, workspaceSlug, sourceNamespace, sourceRevisionDigest), nil
Copy link
Contributor

Choose a reason for hiding this comment

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

We have a proper parser for this - you should reuse that, this format could change over time

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Where's that?

Copy link
Contributor

Choose a reason for hiding this comment

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

speakeasy-api/[email protected]/workflow/source.go

ParseRegistryLocation


// GetRecentWorkspaceGenerations returns the most recent generations of targets in a workspace
// This is based on the CLi event stream, which is updated on every CLI interaction.
func GetRecentWorkspaceGenerations(ctx context.Context) ([]RecentGeneration, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should look to make this smart. We should prioritize targets that are actually connected to Github or are connected to publishing. We may want to consider only including these targets

Once someone is already onboarded we probably don't want people basing new SDKs off of random local generations on someone's machine, I think that will lead to problems because these specs may never be updated again.

}

// The event stream is limited to the most recent 250 events
res, err := speakeasyClient.Events.SearchWorkspaceEvents(ctx, operations.SearchWorkspaceEventsRequest{
Copy link
Contributor

@ryan-timothy-albert ryan-timothy-albert Oct 18, 2024

Choose a reason for hiding this comment

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

You could consider getWorkspaceTargets here instead. Aggregate on unique targets rather than just by events. All 250 events could be more the same target and that target may not be useful.

} else if useRemoteSource && selectedRegistryUri != "" {
// The workflow file will be updated with a registry based input like:
// inputs:
// - location: registry.speakeasyapi.dev/speakeasy-self/speakeasy-self/petstore-oas@latest
Copy link
Contributor

@ryan-timothy-albert ryan-timothy-albert Oct 18, 2024

Choose a reason for hiding this comment

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

Using latest does have some risk given it's written to locally and in CI. All of a sudden a customer could have published SDKs that are being generated based off of local executions of another SDK without them knowing.

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.

4 participants