tilt-dev / tilt

Define your dev environment as code. For microservice apps on Kubernetes.
https://tilt.dev/
Apache License 2.0
7.6k stars 299 forks source link

feat(extensions): auto registration and repo subpaths #6371

Closed avidal closed 4 months ago

avidal commented 4 months ago

note: this is a ground-up rework of a previous PR for nested extensions: #6363

This patch introduces support for specifying load_host and git_subpath when registering an extension repository, each of which influences extension loading.

load_host

If an extension repo has a load_host specified, then extension discovery will attempt to locate an extension repository for an extension whose path starts with a matching load_host value unless the extension is otherwise already registered.

For example:

v1alpha1.extension_repo(name="custom", load_host="internal")

load("ext://internal/abc", "...")

In this case, the extension loading mechanism will first look for an already registered extension by that name, and if not found, will look for an extension repository with a load_host of internal, and then proceed to register the extension for that repo.

The roughly equivalent v1alpha1.extension declaration would be: v1alpha1.extension(name="internal_abc", repo="custom", ...)

git_subpath

In addition to a repo-wide auto registration host this patch introduces a git_subpath argument for non-file based extension repositories. This acts similarly to the repo_path argument for an extension, except it applies to all extensions located in that repository.

For example, combining load_host and git_subpath:

v1alpha1.extension_repo(name="custom", url="...", load_host="internal", git_subpath="extensions")
v1alpha1.extension(name="custom-root", repo_name="custom")
v1alpha1.extension(name="explicit", repo_name="custom", repo_path="explicit")

# Loads from custom:extensions/Tiltfile since the extension has no `repo_path`
load("ext://custom-root", "...")

# Loads from custom:extensions/explicit due to the extension being explicitly registered
load("ext://explicit", "...")

# Loads from custom:extensions/abc due to the registered `internal` load_host
load("ext://internal/abc", "...")

# Loads from custom:extensions/abc/def
load("ext://internal/abc/def", "...")

# Loads the tests/golang extension from the default extension repository
load("ext://tests/golang, "...")

Why?

At Datadog, we have a large Tilt extension library and most of our codebase is in a monorepo. The current extension loading mechanism makes it difficult to organize our extensions in such a way that's both maintainable by the extension authors and not onerous for the users.

These two changes provide a lot more flexibility for managing extension libraries. Using a repo wide subpath allows you to keep your extensions out of the repository root, and registering a load host allows your users to just drop a single extension_repo at the top of their Tiltfile.

With this, our complex extension meta loading extension can be reduced to a single v1alpha1.extension_repo line, and our users can load("ext://tiltlib/...")

avidal commented 4 months ago

For reviewers: note that each commit is logically separate:

avidal commented 4 months ago

Also: if repo.path is too far (or too complex) I think that just having a repo prefix would mostly solve our needs and I can back out that part.

nicks commented 4 months ago

(not sure what's going on with the compose v1 tests, lemme handle that in a separate pr)

avidal commented 4 months ago

(not sure what's going on with the compose v1 tests, lemme handle that in a separate pr)

Thanks, yeah. I looked at the CI logs and couldn't make heads or tails of it. I was just going to rerun and see if it fixed itself :)

nicks commented 4 months ago

ya, if you rebase, the Compose error should go away

avidal commented 4 months ago

@nicks latest version is rebased and has a few more bits and bobs:

There's a few things I'd like to get your opinion on that I'll call out on specific file comments.

avidal commented 4 months ago

It appears the codegen check is failing in CI because of a field name mismatch (the triggering rule is names_match seemingly due to using LoadHost and load_host instead of LoadHost and loadHost.

Curiously, the ExtensionSpec uses RepoPath with a json and protobuf key of repoPath (to fit the name_match rule), but somehow the generated Starlark uses repo_path.

edit: Currently trying a rerun of the codegen after changing the json/protobuf field names to satisfy the rule. I have a hunch that the starlark codegen in-use translates camelCase to snake_case.

edit2: That does not seem to be the case. Changing field names to satisfy the rule results in the starlark API generating using camelCase.

edit3: It did work, I just misread the results of running codegen again.