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

feat: license management feature flag #458

Merged
merged 9 commits into from
Feb 11, 2025

Conversation

christophenne
Copy link
Member

@christophenne christophenne commented Feb 10, 2025

Unfortunately I couldn't make an enum array work with pgx, so I opted for the boolean for now. We can still revisit/refactor/migrate on the second, or latest, third occurrence of a feature flag.

As a positive side effect, I think the manual updating of a db entry will be more straight forward for now 🤷‍♂️

@christophenne christophenne requested a review from kosmoz February 10, 2025 14:43
pmig
pmig previously requested changes Feb 10, 2025
Copy link
Member

@pmig pmig left a comment

Choose a reason for hiding this comment

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

I'd rather prefer to not use a boolean but store features with an ENUM in a list / separate table.

@kosmoz
Copy link
Member

kosmoz commented Feb 11, 2025

I think I found the problem, and it appears to be a bug in pgx!!

I think if you replace LoadTypes(...) and RegisterTypes(...) in the svc package with a loop around LoadType(...) and RegisterType(...) it should work!

@kosmoz
Copy link
Member

kosmoz commented Feb 11, 2025

Debugging the createDBPool function … it appears that the pgTypes returned by conn.LoadTypes is just an empty slice lol

@kosmoz
Copy link
Member

kosmoz commented Feb 11, 2025

I created the following related issue in the pgx repo: jackc/pgx#2254

For now it's probably easiest to just load each type individually.

@christophenne christophenne requested a review from pmig February 11, 2025 12:25
Copy link
Member

@kosmoz kosmoz left a comment

Choose a reason for hiding this comment

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

looks good, just one small question

Signed-off-by: christophenne <[email protected]>
Copy link
Member

@kosmoz kosmoz left a comment

Choose a reason for hiding this comment

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

looks good, but let's merge #455 fist because of the migration IDs

@kosmoz kosmoz dismissed pmig’s stale review February 11, 2025 12:51

issues resolved

@christophenne christophenne merged commit 1f3fede into main Feb 11, 2025
6 checks passed
@christophenne christophenne deleted the christophenne/gk-518-organization-feature-flags branch February 11, 2025 13:27
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.

3 participants