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

Rework the DB api access #62

Merged
merged 5 commits into from
Apr 15, 2024
Merged

Rework the DB api access #62

merged 5 commits into from
Apr 15, 2024

Conversation

ksol
Copy link
Contributor

@ksol ksol commented Apr 12, 2024

per-commit review is slightly more readable.

This PR reworks the db api architecture, mostly by removed the "global" access there was. Before this PR, the DB Api was exposed at the same "level" of other API clients, but it's not a good fit: all other APIs require authenticating as a user (except for guest endpoints), while the DB api require authenticating as a user then as a specific database.

To accomodate this change, api clients need to keep track of which region they're targetting (this allows a regional API client to instantiate the db api client in the same region; the response for token doesn't relay that information).

Slight rename of files/classes as well.

@ksol ksol self-assigned this Apr 12, 2024
Copy link
Contributor

@aurelien-reeves-scalingo aurelien-reeves-scalingo 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

Update of the Readme is missing

And an entry in the unreleased section of the changelog could be nice as this is a huge breaking change.

@ksol
Copy link
Contributor Author

ksol commented Apr 15, 2024

Updated the changelog. Can the readme wait a little bit? I have other PRs incoming and would rather change it all at once

@ksol ksol enabled auto-merge (squash) April 15, 2024 09:14
Copy link
Contributor

@aurelien-reeves-scalingo aurelien-reeves-scalingo left a comment

Choose a reason for hiding this comment

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

It's important not to forget about it as this is a pretty big change.
I think it could even be nice to have an "upgrade guide"

@ksol ksol merged commit 2574d52 into master Apr 15, 2024
6 checks passed
@ksol ksol deleted the feat/redo-db-api branch April 15, 2024 09:17
@ksol
Copy link
Contributor Author

ksol commented Apr 15, 2024

I think it could even be nice to have an "upgrade guide"

If there's a demand for it. The download statistics of the gem are low enough that it might be worth spending the time on it

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