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

Support custom installation path for Docker mode #163

Merged
merged 5 commits into from
Dec 13, 2024

Conversation

johnstairs
Copy link
Member

@johnstairs johnstairs commented Dec 12, 2024

Allow specifying the installation path for Docker mode.

@johnstairs johnstairs marked this pull request as ready for review December 12, 2024 20:39
as query parameters (after the `?`). These are:
your SSH config file (~/.ssh/config). The API socket path can be omitted if the
socket path is the default `/opt/tyger/api.sock` or if the `TYGER_SOCKET_PATH`
environment variable is set on the SSH host.
Copy link
Member Author

Choose a reason for hiding this comment

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

We could also use the environment variable on the client. Thoughts?

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 that might be getting too clever about it, since one could have a local docker install in addition to the remote. I think it is fine the way it is. Is it possible to specify TYGER_SOCKET_PATH as one of the key value pairs? Then what happens if they are not consistent? I am assuming it is not possible, so a non-issue.

Copy link
Member Author

Choose a reason for hiding this comment

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

It is not currently possible to specify it as a key-value pair. Instead, you give the path to the socket as the path in the URL.

@johnstairs johnstairs changed the title [Draft] Support custom installation path for Docker mode Support custom installation path for Docker mode Dec 12, 2024
@hansenms hansenms requested a review from Copilot December 12, 2024 23:56
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 6 out of 21 changed files in this pull request and generated 3 suggestions.

Files not reviewed (15)
  • .devcontainer/devcontainer.json: Language not supported
  • .devcontainer/prepare-host.sh: Language not supported
  • .vscode/launch.json: Language not supported
  • Makefile.docker: Language not supported
  • install/.gitignore: Language not supported
  • scripts/get-config.sh: Language not supported
  • scripts/run-ssh-tests.sh: Language not supported
  • cli/integrationtest/migrations_test.go: Evaluated as low risk
  • cli/internal/client/client.go: Evaluated as low risk
  • cli/internal/client/client_test.go: Evaluated as low risk
  • cli/internal/client/sshurl.go: Evaluated as low risk
  • cli/internal/cmd/stdioproxy.go: Evaluated as low risk
  • cli/internal/controlplane/login.go: Evaluated as low risk
  • cli/internal/install/dockerinstall/docker.go: Evaluated as low risk
  • cli/internal/install/dockerinstall/migrations.go: Evaluated as low risk

server/ControlPlane/Compute/Docker/Docker.cs Show resolved Hide resolved
server/ControlPlane/Buffers/Buffers.cs Show resolved Hide resolved
Copy link
Contributor

@hansenms hansenms left a comment

Choose a reason for hiding this comment

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

Some stuff from my copilot reviewer buddy, but otherwise, works as advertised.

@johnstairs johnstairs merged commit 616ab83 into main Dec 13, 2024
16 checks passed
@johnstairs johnstairs deleted the johnstairs/custom-installation-path branch December 13, 2024 00:41
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.

2 participants