-
Notifications
You must be signed in to change notification settings - Fork 27
[DPE-7302] Prefixes #1238
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
base: 16/edge
Are you sure you want to change the base?
[DPE-7302] Prefixes #1238
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 16/edge #1238 +/- ##
===========================================
- Coverage 70.73% 70.02% -0.72%
===========================================
Files 16 16
Lines 4043 4134 +91
Branches 629 654 +25
===========================================
+ Hits 2860 2895 +35
- Misses 991 1032 +41
- Partials 192 207 +15 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| if self.charm.unit.is_leader(): | ||
| return json.loads(self.charm.app_peer_data.get("rel_databases", "{}")) | ||
|
|
||
| def _get_credentials(self, event: DatabaseRequestedEvent) -> tuple[str, str] | None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Split off, to reduce McCabe.
| return | ||
| return f"relation-{event.relation.id}", new_password() | ||
|
|
||
| def _collect_databases( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Split off, to reduce McCabe.
| self.add_database_to_prefix_mapping(database) | ||
| return database, databases | ||
|
|
||
| def _are_units_in_sync(self) -> bool: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Split off, to reduce McCabe.
| if event.prefix_matching and event.prefix_matching != "all": | ||
| logger.warning("Only all prefix matching is supported") | ||
| databases = sorted(self.charm.postgresql.list_databases(database[:-1])) | ||
| self.set_databases_prefix_mapping(event.relation.id, user, database[:-1], databases) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Set the initial prefixed DB cache.
| else: | ||
| databases = [database] | ||
| # Add to cached field to be able to generate hba rules | ||
| self.add_database_to_prefix_mapping(database) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check and append to existing prefix caches.
| self.database_provides.delete_relation_data( | ||
| relation_id, ["uris", "read-only-uris"] | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On prefix DB that matches nothing, unset the URIs, since there's no possible valid value.
| username: str | ||
| prefix: str | ||
| databases: list[str] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cached prefix metadata. We need to know the username to update Postgresql roles and connect grant, we need to know the prefix, to match against when adding database.
| (dbs := self.get_rel_to_db_mapping()) | ||
| and (database := dbs.get(str(event.relation.id))) | ||
| and database[-1] != "*" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use peer data cache of rel to DBs, since at this point the relation data (and the db name in it) should be gone.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
marceloneppel
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
| (dbs := self.get_rel_to_db_mapping()) | ||
| and (database := dbs.get(str(event.relation.id))) | ||
| and database[-1] != "*" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
Add support for prefix databases.
Depends on:
Checklist