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

Fix relation data access #163

Closed zmraul closed 2 months ago

zmraul commented 2 months ago

Copying from a comment:

Passing self to getattr means that none of those fields on the array exist on the inheritance tree from KafkaRequiresEventHandlers. Because of this, the lib would always write an empty string to databag, so topic_requested is never triggered.

Using the full KafkaRequires class works because we pass data indirectly to via KafkaRequiresEventHandlers.__init__(self, charm, self) The 3rd argument is a self reference, so KafkaRequiresData fields are available down the line.

NOTE: alternative, less intrusive fix

relation_data = {
    f: getattr(self.relation_data, f.replace("-", "_"), "")
    for f in ["consumer-group-prefix", "extra-user-roles", "topic"]
}
zmraul commented 2 months ago

@welpaolo This branch is being merged and based on the one with the test. So it's already passing here :)

@deusebio Once this PR is merged on the test branch, I can rebase on main and that should be addressed no?

deusebio commented 2 months ago

Yes, but probably when rebasing you will have conflicts, given we are touching the same lines. Just remember to keep the spelling consistent! :D

Anyhow, the changes proposed in the PR, looks good to me!