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

tiltfile: pass TILT_HOST + TILT_PORT to local() commands #4773

Merged
merged 4 commits into from Jul 19, 2021

Conversation

milas
Copy link
Contributor

@milas milas commented Jul 19, 2021

Various Tilt extensions have been using local() to shell back
out to Tilt to use the API (e.g. tilt apply -f ...). This does
not play nice with custom ports if Tilt was invoked using the
--port argument.

Now, the current Tilt instance's port is passed as the TILT_PORT
argument so that the invoked command will automatically connect to
the same instance of Tilt.

Fixes #4737.

Various Tilt extensions have been using `local()` to shell back
out to Tilt to use the API (e.g. `tilt apply -f ...`). This does
not play nice with custom ports if Tilt was invoked using the
`--port` argument.

Now, the current Tilt instance's port is passed as the `TILT_PORT`
argument so that the invoked command will automatically connect to
the same instance of Tilt.
@milas milas requested review from nicks and landism July 19, 2021 16:45
internal/tiltfile/files.go Outdated Show resolved Hide resolved
// if Tilt was invoked with `tilt up --port=XXXXX`, local() calls to use the Tilt API will fail due to trying to
// connect to the default port, so explicitly populate the TILT_PORT environment variable if it isn't already
hasTiltPort := false
for _, e := range c.Env {
Copy link
Member

Choose a reason for hiding this comment

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

it might make sense to do this in PrepareEnv with the other Env injections?
https://github.com/tilt-dev/tilt/blob/master/pkg/logger/env.go#L19

though I guess you'd have do pass webport to that function...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I thought about that but it seemed weird since that's logger.PrepareEnv and pretty specific to xterm type stuff.

I (very briefly) looked at doing it in Execer and having local() use that but Execer is way more oriented to use for the BaDer for daemons and it seemed more headache/risky than it was worth. I think maybe the right thing in the future is to replace the pure localexec.ExecCmd with a struct that does the preparation so that it can hold state of things to inject (bonus: you also wouldn't need to pass a logger every time).

internal/tiltfile/files.go Outdated Show resolved Hide resolved
@milas milas changed the title tiltfile: pass TILT_PORT to local() commands tiltfile: pass TILT_HOST + TILT_PORT to local() commands Jul 19, 2021
@milas milas merged commit b2a9a1f into master Jul 19, 2021
@milas milas deleted the milas/local-tilt-port branch July 19, 2021 21:33
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.

Invoking tilt from local breaks with custom port
3 participants