-
Notifications
You must be signed in to change notification settings - Fork 49
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[wip] facade charm #152
base: main
Are you sure you want to change the base?
[wip] facade charm #152
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would help to see how this code is used to properly evaluate this idea.
It could be an integration (?) test where the programmable charm is used.
(Arguably that's no longer an integration test, rather a functional test perhaps?)
|
it's somewhat similar to https://github.com/canonical/any-charm but less focused on programming the charm itself and more focus on the interfaces |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please add in the PR description how this charm will be used in the context of the charm-relation-interfaces project? We are adding this charm under facade_charm
but nothing else refers to it or uses it.
@@ -0,0 +1,239 @@ | |||
interfaces: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably add a comment to mention that this file is automatically generated and should not be modified.
@@ -0,0 +1,15 @@ | |||
# Copyright 2024 pietro |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# Copyright 2024 pietro | |
# Copyright 2024 Canonical Ltd. |
Co-authored-by: Guillaume Belanger <[email protected]>
resp = requests.get("https://charmhub.io/packages.json") | ||
return resp.json()['packages'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider adding resp.raise_for_status()
;
or possibly configuring the client to automatically raise for bab status values.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OT: this gives me exactly 360 charms packages.
I wonder if that's all there is, or is it some kind of server limit?
|
||
|
||
async def get_all_integrations(charms_pkg_info): | ||
async with aiohttp.ClientSession() as session: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
async with aiohttp.ClientSession() as session: | |
async with aiohttp.ClientSession(raise_for_status=True) as session: |
I would also include a base URL, it's pretty handy.
This PR adds a
facade
charm to this repository.The facade charm is a 'universal prorgrammable relation interface mocking charm'.
Charmcraft.yaml is dynamically generated, and to help people remember that they should regenerate it on each use, it's generated by
tox -e pack
and deleted afterwards.The idea is:
Purpose:
See it in action:
https://github.com/canonical/tempo-k8s-operator/pull/127/files#diff-0b4ff0c5a367926f20bde3d63ac17e8a1b14a6459e6a05e2ec6a0e2c654c1e3c