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

[DPE-3562] Don't block on failure to get the db version #578

Merged
merged 3 commits into from
Aug 12, 2024

Conversation

dragomirp
Copy link
Contributor

@dragomirp dragomirp commented Aug 9, 2024

Conditionally update the db version field in legacy relations if database is not accessible.

Related to #352 and #566

Copy link

codecov bot commented Aug 9, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.84%. Comparing base (623d9a9) to head (398c77e).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #578   +/-   ##
=======================================
  Coverage   70.83%   70.84%           
=======================================
  Files          12       12           
  Lines        3024     3025    +1     
  Branches      535      536    +1     
=======================================
+ Hits         2142     2143    +1     
  Misses        768      768           
  Partials      114      114           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Comment on lines +394 to +395
if postgresql_version:
data["version"] = postgresql_version
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Only add the version if we were able to get it.

@@ -319,14 +319,14 @@ def update_endpoints(self, relation: Relation = None) -> None:
if len(relations) == 0:
return

postgresql_version = None
try:
postgresql_version = self.charm.postgresql.get_postgresql_version()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alternatively we can use cluster.get_postgresql_version instead and get the version from the snap.

Comment on lines +434 to +437
if current_host:
host = self.current_host
else:
host = None
Copy link
Contributor Author

@dragomirp dragomirp Aug 9, 2024

Choose a reason for hiding this comment

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

Use current_host by default, I think we still need to be able to call the primary for pgbouncer.

@dragomirp dragomirp marked this pull request as ready for review August 10, 2024 12:01
Copy link
Contributor

@taurus-forever taurus-forever left a comment

Choose a reason for hiding this comment

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

LGTM! Tnx!

Copy link
Member

@marceloneppel marceloneppel left a comment

Choose a reason for hiding this comment

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

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants