-
Notifications
You must be signed in to change notification settings - Fork 13
feat: add Temporal charm integration libraries #266
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
Conversation
|
@james-garner-canonical is there a way to skip the machine integration tests? These relations are for k8s charms so I don't think it makes sense to have machine integration tests |
Yes, @wyattrees, CI uses You can mark an entire module of tests by putting |
Having thought about this more and talked it over with the team, it seems like the library doesn't have any k8s specific requirements or anything, so what do you think about just testing on both substrates? It doesn't look like the test charms would need to differ much, so it shouldn't lead to much duplication. |
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.
Thanks for this PR, @wyattrees! It's really nice to see new interface libraries being created as Python packages here instead of as Charmhub-hosted libs.
I have only skimmed the library code, but I just wanted to start by requesting a bit more documentation about the interfaces.
You've caught us just as we've migrated all the interface definitions from charm-relation-interfaces to this repo, but before we've updated the contribution documentation to account for this.
It would be great if you could add the following for each of interfaces/temporal-host-info and interfaces/temporal-worker-consumer:
interface/v0/interface.yaml
Note: edited to separate summary and description.
name: # interface name that will be used in charmcraft.yaml
summary: # short one sentence summary to be displayed in charmhub docs/etc, like "Request endpoints for pushing trace data."
description: # longer multi-sentence overview of the purpose and function of the interface
version: 0
status: # draft if this should be considered unstable (consider 0.X release of lib if so), otherwise published
lib: # charmlibs.interfaces.underscored_interface_name
maintainer: # presumably your team name on Github
providers: # list the charms that you know will be using this lib
- name: # charm name on charmhub
url: # https://github.com/canonical/...
requirers: # as providersinterface/v0/README.md
This readme documents the interface contract, and will be hosted on the documentation website separately from the library docs -- these docs are for library implementers and low-level debugging, while the library docs are for your library's users. You can follow the format used here. Make sure to also mention the recommended library (these ones) and the charming solution this is being designed for (e.g. link to temporal server charm docs).
I notice that the library hasn't defined the databag contents in schemas, which is perhaps fair since they're pretty lightweight. Let me follow up with you about this tomorrow, we're working out some recommendations for interface libraries on this front.
Could you also please update the CODEOWNERS file with the owners for these new libraries?
|
Per discussion with @wyattrees, the team have decided not to publish these interface libraries for now, so I'll close this PR. It can definitely be reopened if plans change. |
This adds two new interface libraries,
temporal_worker_consumerandtemporal_host_infofor upcoming changes to the Temporal charm suite to include these new relations.