fix: obtain Postgres URL from branches get command by yaten2302 · Pull Request #2996 · supabase/cli

yaten2302 · 2024-12-25T18:05:11Z

What kind of change does this PR introduce?

This PR introduces a new feature to enable users to obtain the Postgres db url from the .env using the branches get command.

What is the current behavior?

Fixes #2964

What is the new behavior?

This PR adds a new flag to the branches get command to help users to get the postgres db url from the env.

Additional context

Add any other context or screenshots.

Signed-off-by: Yaten Dhingra <[email protected]>

yaten2302 · 2024-12-25T18:17:14Z

@sweatybridge , I've a doubt, that as you mentioned in this comment, that a new --output flag should be added to the branches get command. But, while implementing that, it's showing an error that the output flag is being redeclared. So, is it possible that instead of using the output flag, we can use a Boolean var postgres-url?

Also, as you mentioned, that the postgres_url and prostgres_url_non_pooling has to be added if this flag is mentioned with the command and the code can be reused from the bootstrap command. But, while I was checking the writeDotEnv func, it writes the complete thing to the .env file. But, I'm confused that how can I get the postgres_db_url from the writeDotEnv func? Because, I was having some issues in adding the postgres_db_url to the resp, could you guide that how can I add the same to the resp, so that it can be used to display it on the output table for the user.

Signed-off-by: Yaten Dhingra <[email protected]>

yaten2302 · 2024-12-31T18:15:40Z

Hey @sweatybridge , could you please have a look at this PR, I've made the required changes, kindly review and LMK if any more are required!!
Thanks

coveralls · 2024-12-31T18:18:36Z

Pull Request Test Coverage Report for Build 12618705729

Details

0 of 25 (0.0%) changed or added relevant lines in 2 files are covered.
6 unchanged lines in 2 files lost coverage.
Overall coverage decreased (-0.1%) to 59.599%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
pkg/cast/cast.go	0	5	0.0%
internal/branches/get/get.go	0	20	0.0%

Files with Coverage Reduction	New Missed Lines	%
internal/branches/get/get.go	1	0.0%
internal/gen/keys/keys.go	5	12.9%

Totals
Change from base Build 12595878677:	-0.1%
Covered Lines:	7668
Relevant Lines:	12866

💛 - Coveralls

Signed-off-by: Yaten Dhingra <[email protected]>

sweatybridge · 2025-01-01T06:13:30Z

Let's do this in smaller steps so I can provide more detailed guidance.

But, while implementing that, it's showing an error that the output flag is being redeclared.

This is how golang works: any variable declared in the same package is shared by all code in that package. Since the output variable is already declared in cmd package, you don't need to redeclare for this command.

For your first commit, simply passing it down to branches get command will do. No need to implement the actual behaviour for env output.

After you've done that, tag me again so I can check before providing more details for the actual implementation.

sweatybridge

Feel free to open a new PR if it's easier for you.

sweatybridge · 2025-01-01T06:18:35Z

Also, sorry for the late reply as I was on holiday for the last 2 weeks and there were a lot to catch up on. I will review more promptly this week and next if you plan to make the changes.

Signed-off-by: Yaten Dhingra <[email protected]>

yaten2302 · 2025-01-01T06:35:50Z

Heyy @sweatybridge , sorry, actually, I was pushing some changes right now only and that's why didn't see your comment on this PR.

Let's do this in smaller steps so I can provide more detailed guidance.

But, while implementing that, it's showing an error that the output flag is being redeclared.

This is how golang works: any variable declared in the same package is shared by all code in that package. Since the output variable is already declared in cmd package, you don't need to redeclare for this command.

For your first commit, simply passing it down to branches get command will do. No need to implement the actual behaviour for env output.

After you've done that, tag me again so I can check before providing more details for the actual implementation.

Also, yes, for this, I was also researching about the same and now I understood that, vars are shared across the same package files in Go.
I've made some changes now, I think that these might fix this --output-env flag. Could you have a look at it, whenever you're free!!
Thanks :)

cmd/branches.go

Signed-off-by: Yaten Dhingra <[email protected]>

cmd/branches.go

Co-authored-by: Han Qiao <[email protected]>

yaten2302 · 2025-01-01T08:00:59Z

Now, how should the Run() func be changed in internal/branches/get/get.go in order to get the POSTGRES_URL_NON_POOLING from the internal/bootstrap/bootstrap.go?

cmd/branches.go

internal/branches/get/get.go

Co-authored-by: Han Qiao <[email protected]>

internal/branches/get/get.go

Signed-off-by: Yaten Dhingra <[email protected]>

internal/bootstrap/bootstrap.go

internal/branches/get/get.go

cmd/branches.go

internal/branches/get/get.go

Signed-off-by: Yaten Dhingra <[email protected]>

cmd/branches.go

internal/branches/get/get.go

Signed-off-by: Yaten Dhingra <[email protected]>

sweatybridge · 2025-01-04T03:47:43Z

internal/branches/get/get.go

-|-|-|-|-|-|-|-|
+	config := pgconn.Config{
+		Host: utils.GetSupabaseDbHost(resp.JSON200.DbHost),
+		Port: uint16(resp.JSON200.DbPort),


Suggested change

Port: uint16(resp.JSON200.DbPort),

Port: uint16(cast.IntToUint(resp.JSON200.DbPort)),

To fix lint check

@yaten2302 in case you missed it, there are more linter errors to fix https://github.com/supabase/cli/actions/runs/12608039666

@sweatybridge , yes, ig I forgot about the linter errors, I've a doubt, that in here, these 2 errors are being show

In the 2nd one, should I remove cast.IntToUInt(...), I think the error is arising because of that only?
For the first one, I've pushed a commit, I think it should be fixed now (formatted the file)

In the 2nd one, should I remove cast.IntToUInt(...), I think the error is arising because of that only?

We need to implement a new method for safely casting UInt to UInt16 and call it. You can check the existing examples in cast.go on how to check for overflows. But if you feel stuck, I can also take over from here.

I'll research upon it a and then ask if I encounter any doubts! Also, I wanted to ask that, is it possible to check the linter errors are failing or passing without pushing the changes?

Hey, I just now pushed a commit to try to fix the linter error, but unfortunately it's failing, could you please guide that is there some other file, in which some issue similar to this is being handled? I would like to understand from that and then try to implement that here.

// UIntToUInt16 converts a uint to an uint16, handling potential overflow func UIntToUInt16(value uint) int { if value <= math.MaxUInt16 { return uint16(value) } return math.MaxUInt16 }

You want to add the above to cast.go and call it like this

Port: cast.UIntToUInt16(cast.IntToUInt(reps.JSON200.Port))

Oh, okay, got it now, thanks for your help 👍
But, in the first line, shouldn't the return value of the UIntToUInt16 func be uint16?

Signed-off-by: Yaten Dhingra <[email protected]>

yaten2302 · 2025-01-05T10:54:00Z

Thank you so much @sweatybridge for helping me out with this PR, this was my very first contribution with supabase, I'll definitely check out some other issues and work on them 👍

fix:2964/obtain Postgres DB URL from ENV using branches get command

0fe8088

Signed-off-by: Yaten Dhingra <[email protected]>

yaten2302 and others added 2 commits December 31, 2024 20:17

Merge branch 'supabase:develop' into develop

215116f

added --output-env flag in branches get command

1342a93

Signed-off-by: Yaten Dhingra <[email protected]>

yaten2302 marked this pull request as ready for review December 31, 2024 18:14

yaten2302 requested a review from a team as a code owner December 31, 2024 18:14

fixed typo in internal/branches/get/get.go

5f7573b

Signed-off-by: Yaten Dhingra <[email protected]>

sweatybridge requested changes Jan 1, 2025

View reviewed changes

yaten2302 added 2 commits January 1, 2025 11:53

updated the output-env flag in branches.go

7898aca

Signed-off-by: Yaten Dhingra <[email protected]>

changed output var to postgres_url var in branches.go

031d8c2

Signed-off-by: Yaten Dhingra <[email protected]>