Skip to content

Conversation

dragomirp
Copy link
Contributor

@dragomirp dragomirp commented Jan 3, 2023

Issue

  • DPE-1073
  • Low log verbosity when deferring events within the charm
  • DPE-1080
  • Warnings in unit tests

Solution

  • Add debug log messages on event deferrals and early returns
  • Clear up warnings

Context

  • Added log messages within the charm, didn't touch or check library code deferrals
  • Added a patch to a unit test that fails if dpkg is not present on the system
  • Switch to tox ini to use allowlist_externals
  • Enabled SIMULATE_CAN_CONNECT for relevant tests
  • Adding unit test dependency to suppress the asyncio_mode warning config

Testing

  • Increased a bit the unit test coverage

Release Notes

Increased log verbosity on deferrals

@codecov
Copy link

codecov bot commented Jan 3, 2023

Codecov Report

Merging #49 (199a1e6) into main (0073d8f) will increase coverage by 1.29%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main      #49      +/-   ##
==========================================
+ Coverage   64.05%   65.35%   +1.29%     
==========================================
  Files           6        6              
  Lines         818      814       -4     
  Branches      121      115       -6     
==========================================
+ Hits          524      532       +8     
+ Misses        264      258       -6     
+ Partials       30       24       -6     
Impacted Files Coverage Δ
src/charm.py 57.43% <ø> (+1.18%) ⬆️
src/relations/db.py 69.74% <ø> (+3.08%) ⬆️
src/relations/postgresql_provider.py 88.60% <ø> (+1.26%) ⬆️
src/cluster.py 68.00% <0.00%> (+0.21%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@dragomirp dragomirp changed the title DPE 1073: DEBUG verbosity for deferrals and early returns DPE 1073 DPE 1080: DEBUG verbosity for deferrals and early returns Jan 4, 2023
@dragomirp dragomirp marked this pull request as ready for review January 4, 2023 13:26
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. Nice! Tnx!

BTW, how to deal with unhappy codecov?

@dragomirp
Copy link
Contributor Author

dragomirp commented Jan 5, 2023

@taurus-forever To not fail the codecov checks we must cover the specified amount (65%) of the new code and the overall code

@taurus-forever
Copy link
Contributor

@taurus-forever To not fail the codecov checks we must cover the specified amount (65%) of the new code and the overall code

It doesn't match the warnings I see in https://github.com/canonical/postgresql-operator/pull/49/checks?check_run_id=10457437969
As for me codecov explicitly blame new debug lines... Which are hard to cover with unit tests (defer cases).
I hope I am wrong. @marceloneppel any comments? Should enforce such codecov requirements?

@marceloneppel
Copy link
Member

@taurus-forever To not fail the codecov checks we must cover the specified amount (65%) of the new code and the overall code

It doesn't match the warnings I see in https://github.com/canonical/postgresql-operator/pull/49/checks?check_run_id=10457437969 As for me codecov explicitly blame new debug lines... Which are hard to cover with unit tests (defer cases). I hope I am wrong. @marceloneppel any comments? Should enforce such codecov requirements?

I think enforcing testing debug lines is not something that brings value to the code. We probably can use something like the .coveragerc file like the one from https://github.com/canonical/opensearch-operator/blob/56b9ed036d44f69a269217087093c55db89926c8/.coveragerc to avoid some lines to be verified in the coverage checks.

I thought on something like (I haven't tested it):

[report]
exclude_lines =
    # Don't complain about missing debug-only code:
    logger\.debug

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.

Amazing! Thanks for all those improvements @dragomirp!

@dragomirp dragomirp merged commit 362ad57 into main Jan 5, 2023
@dragomirp dragomirp deleted the dpe-1073-debug-logging-verbosity branch January 5, 2023 17:03
BON4 pushed a commit to BON4/postgresql-operator that referenced this pull request Apr 23, 2024
github-actions bot added a commit to canonical/test-runners-2-is-x64-postgresql-operator that referenced this pull request May 24, 2024
github-actions bot added a commit to canonical/test-runners-2-azure-arm64-postgresql-operator that referenced this pull request May 24, 2024
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