feat(sunbeam-python): add FeatureGroup capability - #915
Conversation
|
To be clear, code is by @hemanthnakkina, PR is by me - might not be best practice to add as a reviewer of own code. It is not changed from the initial suggestion at this point. |
|
@mv-2112 Can you add unit tests if you have time otherwise i will look into this |
ccda57a to
33e40b1
Compare
|
Apologies, messed a rebase up. Fixed and squashed. |
gboutry
left a comment
gboutry
left a comment
There was a problem hiding this comment.
Please rebase on main
The attached comment is rather a nit, but if you can just add a comment in the code so it's not lost, it would be appreciated.
|
Looks like this is still conflicting with the base, please rebase. (btw, there's 2 commits in the branch history, not sure that's your goal) And to mention Hemanth, I think we're in a case where he can be mentioned as And please, add |
|
Will revisit whats going on here... local branch is up to date with #919 but throwing this conflict. Whatever has changed means i'm finding it difficult to find where the work Hemanth has done fits. I think Co-Author undersells it - it think its a full blown Authored by. |
7139d80 to
f0503cb
Compare
This is required for Cloudkitty to correctly register its need for observability to be enabled. Unlike telemetry for example, observability has two subfeatures, embedded and external that are both equally valid, but one MUST be provided hence optional is not a valid way to handle in the current setup. Authored-By: Hemanth Nakkina <hemanth.nakkina@canonical.com> Signed-off-by: Matt Verran <verranm76@gmail.com>
gboutry
left a comment
gboutry
left a comment
There was a problem hiding this comment.
Thanks for the contribution!
3 participants
This is required for Cloudkitty to correctly register its need for observability to be enabled. Unlike telemetry for example, observability has two subfeatures, embedded and external that are both equally valid, but one MUST be provided hence optional is not a valid way to handle in the current setup.
Original Author/Inspiration: Hemanth Nakkina
QA steps
We need to ensure this extends the FeatureRequirement without breaking existing.
Since on #581 it passes for telemetry it can serve as a test case for this feature. Telemetry should pass as normal, and additionally observability should now pass.
Links
lp2167079