canonical / data-platform-libs

A collection of charm libraries curated by the Data Platform Team
https://charmhub.io/data-platform-libs
Apache License 2.0
9 stars 7 forks source link

[DPE-4335] Do not trigger secret revision change if the relation content is the same #169

Closed Batalex closed 1 month ago

Batalex commented 1 month ago

This PR prevents the data interface abstraction from triggering the emission of a new secret revision if the content is the same. The behavior is unaltered when using the secret API itself from data_secrets.py

Fixes https://github.com/canonical/data-platform-libs/issues/164

juditnovak commented 1 month ago

Seems as a great change with great tests, thank you very much for it!!!! :-)

I'll get after having verified that this is surely all that was needed to do (as some of the workflows may be a bit "tangled up" :sweat_smile: )

juditnovak commented 1 month ago

The test charms contents are dynamically fetched, we shouldn't keep them updated. (I guess we should rather do a proper cleanup on that at some point :sweat_smile: )

Batalex commented 1 month ago

The test charms contents are dynamically fetched, we shouldn't keep them updated. (I guess we should rather do a proper cleanup on that at some point 😅 )

I can gitignore them if you'd like. I was committing them for consistency intra repo

juditnovak commented 1 month ago

@Batalex I see your point... The reason why I'd advise otherwise is because then we need to maintain this consistency from now on. Normally these files are being copied over from latestt just for the sake of the tests. If we now add them to the repo, we need to update them on every single change...

(There was a specific reason why I added the initial version for dummy-database-charm... I could agree to update that one. But I rather should look into ways to get rid of that copy next time I'm touching the tests :-) Let's not have more of those copies if avoidable...)

juditnovak commented 1 month ago

The PR is all excellent now in terms of code and integration tests... May I just pls ask for a unittest... (Seems like a low-hanging fruit, a simple copy of existing secrets-related tests. If it's more than that, then we can skip.)

Batalex commented 1 month ago

@juditnovak I reverted all changes to test charms libs :saluting_face: