search

canonical / charms.reactive

Framework for developing charms and relations using the reactive pattern
Apache License 2.0
22 stars 34 forks source link

Fixing wrong reference #235

Closed xtrusia closed 2 years ago

xtrusia commented 2 years ago

Fixing wrong reference

relation_id is in Endpoint class, but self.relation. returns relation.

so it should be self.relation_id instead of self.relation.relation_id

Actually I think this haven't used before, as LP[1] is the cause of this implementation. ( prior patch was : https://github.com/juju-solutions/charms.reactive/pull/232 )

Im trying to make patch for app relation data. but prior patch has bug. This bug is not public I think but when I tries to use prior patch, I face issue.

[1] LP : https://bugs.launchpad.net/charm-kubernetes-master/+bug/1899706

ajkavanagh commented 2 years ago

Fixing wrong reference

Please could you supply a bit more context to the PR? e.g. what bug is it fixing? There should also be a unit test to verify the change (particularly if there was no coverage of the code previously).

xtrusia commented 2 years ago

Fixing wrong reference

Please could you supply a bit more context to the PR? e.g. what bug is it fixing? There should also be a unit test to verify the change (particularly if there was no coverage of the code previously).

xtrusia commented 2 years ago

accidently i clicked closed button.. sorry

xtrusia commented 2 years ago

thanks, I removed temp file and re-added py 3.5 for testing.

testing for 3.5 will fail but didn't remove it as you said it is needed for supporting xenial

Thanks.

ajkavanagh commented 2 years ago

Okay, so re: verification, it would be good to see a charm that uses this? e.g. link to charm code that will use this (and probably what you are testing with?) It's just really hard to see how this works without seeing it in use. Thanks!

xtrusia commented 2 years ago

Okay, so re: verification, it would be good to see a charm that uses this? e.g. link to charm code that will use this (and probably what you are testing with?) It's just really hard to see how this works without seeing it in use. Thanks!

hey, it is new feature so please refer to this this code should be merged for fixing my case. I think there is no other charms using this.

https://github.com/juju-solutions/interface-prometheus-manual/pull/6/files

ajkavanagh commented 2 years ago

Okay, so re: verification, it would be good to see a charm that uses this? e.g. link to charm code that will use this (and probably what you are testing with?) It's just really hard to see how this works without seeing it in use. Thanks!

hey, it is new feature so please refer to this this code should be merged for fixing my case. I think there is no other charms using this.

https://github.com/juju-solutions/interface-prometheus-manual/pull/6/files

Okay, let's finish this off; if you strip out the py3.5 github workflow test (as it's no longer supported, as you originally discovered), then we'll merge this. Thanks.