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.
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(starter): starter improvements to avoid port clashes #505
feat(starter): starter improvements to avoid port clashes #505
Changes from all commits
207d94a
b0ed4b9
ecd5dbd
ecbe5bd
17d2201
6e3e48b
5e44cf6
ef5d3af
2461291
ac84395
5d2d45a
7efb27b
cb43e50
020684d
4493c53
af81cdb
17b0705
cf4cc52
0e18721
d1f44dd
a8764b0
bd0a60f
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we still need to expose this port?
The compose file is not binding to 5433 so i'm assuming it is not necessary here either and we can remove it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
5433 is the logical publisher port. I think it is being exposed here to support setup where people are running their own postgres on the host while Electric itself is run inside a Docker container.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we do need to expose it, because we need the PG to be able to connect to the electric. In this case, isn’t the script for when you’re running Electric without Postgres in the same compose?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh yes, i see. Perhaps i'll leave a comment to remember.
No the script for running only Electric is this separate file that executes
docker run
because there are some differences (PG url can be passed as an argument, and 5433 is not exposed in compose file).