This repository was archived by the owner on Sep 30, 2024. It is now read-only.
app: resolve port :9000 conflicts (change to :49000) #54466
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Stephen Gutekanst <stephen@sourcegraph.com>
|
Codenotify: Notifying subscribers in CODENOTIFY files for diff 79bddb9...87dde75.
|
Strum355
approved these changes
Jun 30, 2023
|
This looks like it shouldn't be an issue, but could we get a startup on |
|
Good catch, I meant to list that in the test plan - will update! |
burmudar
approved these changes
Jun 30, 2023
emidoots
pushed a commit
that referenced
this pull request
Jul 4, 2023
…main' CI) We still use `sourcegraph/server` to run our integration tests in our CI pipelines (yuck) and in my change #54466 I failed to realize that `IsSingleBinary()` and `IsDeployTypeSingleDockerContainer(Type())` report different things. This fixes the issue, and likely fixes `main` on our CI pipelines. Signed-off-by: Stephen Gutekanst <stephen@sourcegraph.com>
emidoots
pushed a commit
that referenced
this pull request
Jul 4, 2023
…main' CI) (#54601) We still use `sourcegraph/server` to run our integration tests in our CI pipelines (yuck) and in my change #54466 I failed to realize that `IsSingleBinary()` and `IsDeployTypeSingleDockerContainer(Type())` report different things. This fixes the issue, and likely fixes `main` on our CI pipelines. If not, then both this change and the other should be reverted. ## Test plan CI Signed-off-by: Stephen Gutekanst <stephen@sourcegraph.com>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Apparently VS Code's Python support uses port 9000, so as long as we use it most VS Code Python devs can't run the app due to the port conflict with our services.
This PR switches the app's blobstore to listen on
:49000instead of:9000. My thinking for now is that we can centralize all "default endpoint" and "what host/port should I listen on?" into thedeploypackage, and for now just change ports to have a4in front of them to reduce the change of conflicts.Later, once we have them centralized in the
deploypackage we can easily modify that code to pick an unavailable port.This also binds blobstore to
127.0.0.1(more secure), previously it was bound tolocalhost.Test plan
To test this I did the following:
9000and vetted each instance, to ensure I didn't miss anything.sg start appand confirmedsudo lsof -i -P | grep LISTEN | grep :9000reported nothing whilesudo lsof -i -P | grep LISTEN | grep :49000did../enterprise/dev/app/build.shand tested it:sg start enterprise-codeintelstarts and runs