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

Rename "Postgres nodes" in control_plane to endpoints. #3938

Merged
merged 1 commit into from
Apr 13, 2023

Conversation

hlinnaka
Copy link
Contributor

@hlinnaka hlinnaka commented Apr 3, 2023

We use the term "endpoint" in for compute Postgres nodes in the web UI
and user-facing documentation now. Adjust the nomenclature in the code.

This changes the name of the "neon_local pg" command to "neon_local
endpoint". Also adjust names of classes, variables etc. in the python
tests accordingly.

This also changes the directory structure so that endpoints are now
stored in:

.neon/endpoints/<endpoint id>

instead of:

.neon/pgdatadirs/tenants/<tenant_id>/<endpoint (node) name>

The tenant ID is no longer part of the path. That means that you
cannot have two endpoints with the same name/ID in two different
tenants anymore. That's consistent with how we treat endpoints in the
real control plane and proxy: the endpoint ID must be globally unique.

(This is extracted to separate PR from PR #3886, and developed further to also rename the 'neon_local pg' command to 'neon_local endpoint', and all the changes in the python test)

@hlinnaka hlinnaka requested review from a team, lubennikovaav and LizardWizzard and removed request for a team April 3, 2023 13:58
@hlinnaka hlinnaka force-pushed the neon_local-endpoints branch 7 times, most recently from 56bf5b4 to da73f68 Compare April 4, 2023 13:04
@hlinnaka hlinnaka requested review from a team and NanoBjorn and removed request for a team April 4, 2023 13:04
@github-actions
Copy link

github-actions bot commented Apr 4, 2023

Test results for 1ce670a:


debug build: 212 tests run: 202 passed, 0 failed, 10 (full report)


release build: 212 tests run: 202 passed, 0 failed, 10 (full report)


@hlinnaka hlinnaka force-pushed the move-compute-spec branch from b410af5 to 8e06018 Compare April 5, 2023 16:49
@hlinnaka hlinnaka force-pushed the neon_local-endpoints branch from da73f68 to 1f2946a Compare April 5, 2023 17:04
@hlinnaka hlinnaka force-pushed the move-compute-spec branch from 8e06018 to 939ea29 Compare April 6, 2023 20:25
@hlinnaka hlinnaka force-pushed the neon_local-endpoints branch from 1f2946a to 1326ccc Compare April 6, 2023 20:28
@hlinnaka hlinnaka force-pushed the move-compute-spec branch 2 times, most recently from 1f2df18 to f0b2e07 Compare April 9, 2023 18:54
@hlinnaka hlinnaka force-pushed the neon_local-endpoints branch from 1326ccc to d823c15 Compare April 9, 2023 19:01
Base automatically changed from move-compute-spec to main April 9, 2023 19:12
@hlinnaka hlinnaka changed the base branch from main to spec-cleanups April 12, 2023 07:00
@hlinnaka hlinnaka force-pushed the neon_local-endpoints branch from d823c15 to 50e38f5 Compare April 12, 2023 07:01
Base automatically changed from spec-cleanups to main April 12, 2023 09:11
@hlinnaka hlinnaka force-pushed the neon_local-endpoints branch from 50e38f5 to 7f4f29e Compare April 12, 2023 09:14
Copy link
Member

@koivunej koivunej left a comment

Choose a reason for hiding this comment

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

The biggest problem is mental migration, and maybe someone has scripts. We could assist with that while accumulating 1 line of code to make a question on slack unnecessary.

Aside from these ... I think it looks good if the tests pass? :) I can't say I read through everything.

control_plane/src/bin/neon_local.rs Show resolved Hide resolved
@hlinnaka hlinnaka force-pushed the neon_local-endpoints branch from 7f4f29e to 4c20605 Compare April 13, 2023 09:33
Copy link
Member

@koivunej koivunej left a comment

Choose a reason for hiding this comment

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

Went through the control_plane and fixtures parts again, couldn't find anything apart from the rename and minor refactorings.

@koivunej
Copy link
Member

The regress test failure was a #2962 I think:

AssertionError: assert 25821184 == 25812992
  +25821184
  -25812992

We use the term "endpoint" in for compute Postgres nodes in the web UI
and user-facing documentation now. Adjust the nomenclature in the code.

This changes the name of the "neon_local pg" command to "neon_local
endpoint". Also adjust names of classes, variables etc. in the python
tests accordingly.

This also changes the directory structure so that endpoints are now
stored in:

    .neon/endpoints/<endpoint id>

instead of:

    .neon/pgdatadirs/tenants/<tenant_id>/<endpoint (node) name>

The tenant ID is no longer part of the path. That means that you
cannot have two endpoints with the same name/ID in two different
tenants anymore. That's consistent with how we treat endpoints in the
real control plane and proxy: the endpoint ID must be globally unique.
@hlinnaka hlinnaka force-pushed the neon_local-endpoints branch from 4c20605 to 1ce670a Compare April 13, 2023 10:46
@hlinnaka hlinnaka merged commit 53f438a into main Apr 13, 2023
@hlinnaka hlinnaka deleted the neon_local-endpoints branch April 13, 2023 11:34
@hlinnaka
Copy link
Contributor Author

Pushed, thanks for the review, @koivunej !

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