Skip to content

cache external postgres port in Branch model#674

Merged
boddumanohar merged 3 commits intodevfrom
issue-181
Mar 31, 2026
Merged

cache external postgres port in Branch model#674
boddumanohar merged 3 commits intodevfrom
issue-181

Conversation

@boddumanohar
Copy link
Copy Markdown
Member

@boddumanohar boddumanohar commented Mar 19, 2026

fixes: simplyblock/vela#181

Currently as a part of _public of branch the node_port of a service is retrieved from API server. But this port number never changes. So this can be safely stored in branch model.

testing

after migration upon calling the /branch endpoint, the DB got populated

postgres=# select db_port from branch;
 db_port 
---------
        
        
(2 rows)

postgres=# select db_port from branch;
 db_port 
---------
   31432
   31444
(2 rows)

postgres=# 

@boddumanohar boddumanohar marked this pull request as draft March 19, 2026 05:45
@boddumanohar boddumanohar force-pushed the issue-181 branch 2 times, most recently from 76fb871 to 35de377 Compare March 19, 2026 11:41
Base automatically changed from dev to main March 19, 2026 14:34
@boddumanohar boddumanohar changed the base branch from main to dev March 20, 2026 12:04
Base automatically changed from dev to main March 25, 2026 12:30
@boddumanohar boddumanohar changed the base branch from main to dev March 25, 2026 13:28
@boddumanohar boddumanohar marked this pull request as ready for review March 31, 2026 07:19
@boddumanohar boddumanohar requested a review from mxsrc March 31, 2026 07:19
Copy link
Copy Markdown
Collaborator

@mxsrc mxsrc left a comment

Choose a reason for hiding this comment

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

I think a cleaner approach would be a single helper to access the port, returning the branch attribute if present, if not, looking up and persisting the port. Ideally, it'd fail when the lookup fails as well.

This would avoid the fallbacks on access, avoid having to call the _persist function in multiple places, which is a code smell, and avoid having to establish the values during the migration.

@boddumanohar
Copy link
Copy Markdown
Member Author

thanks, that's actually a great idea. I've implemented the changes and update the PR description with the testing results.

mxsrc
mxsrc previously approved these changes Mar 31, 2026
@boddumanohar boddumanohar merged commit ccf4b47 into dev Mar 31, 2026
7 of 8 checks passed
@boddumanohar boddumanohar deleted the issue-181 branch March 31, 2026 14:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants