-
Notifications
You must be signed in to change notification settings - Fork 3
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
feat: Implements httprequest lego k8s operator (do not merge) #27
Conversation
|
||
integration-test: | ||
name: Integration tests | ||
runs-on: ubuntu-22.04 |
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.
please add some requirements for this workflow. Integration tests hog CI resources during peak hours so best to run this when the other reqs are met
needs:
- lint-report
- static-analysis
- unit-tests-with-coverage
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.
PR #29 addresses this
@@ -0,0 +1,43 @@ | |||
name: Promote Charm | |||
|
|||
on: |
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 like this workflow file. I've actually never seen it before.
Question: is anyone authorised to run this workflow? Might be good to set this to management / senior eng
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.
Everyone in the team yes as the whole team is admin on this github project, but this is configurable through github permission.
@@ -0,0 +1,54 @@ | |||
name: Auto-update Charm Libraries | |||
on: |
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 like this too
description: URL of the HTTP endpoint to use | ||
httpreq_http_timeout: | ||
type: int | ||
description: API request timeout |
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.
might be nice to include the measurement s/m/h since this will be shown on charmcraft under Reference
description: Basic authentication password | ||
httpreq_polling_interval: | ||
type: int | ||
description: Time between DNS propagation check |
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.
same here
httpreq_polling_interval: | ||
type: int | ||
description: Time between DNS propagation check | ||
httpreq_propagation_timeout: |
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.
and here
@@ -0,0 +1,4 @@ | |||
ops |
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.
not needed for publishing, but might be nice to have versions or incorporate poetry tool
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 are in the process of implementing pip-tools
in order to achieve the same purpose (freezing dependencies). This project doesn't have this yet though.
raise_on_blocked=True, | ||
timeout=1000, | ||
) | ||
assert ops_test.model.applications[APP_NAME].units[0].workload_status == "active" |
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.
One of the requirements for publishing is for every build, it is ensured that the integration with other charms is successful.
I would suggest that the charm :
- can be integrated with other charms
- the integrated charm receives the expected certs
website: https://charmhub.io/httprequest-lego-k8s | ||
source: https://github.com/canonical/httprequest-lego-k8s-operator | ||
issues: https://github.com/canonical/httprequest-lego-k8s-operator/issues | ||
docs: https://discourse.charmhub.io/t/http-request-acme-operator-docs-index/12513 |
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.
:)
Description
The goal of this PR is to help in the process of having this charm listed in Charmhub following the charm review process. Here is the link to the charm review request.
Please do not merge this PR.
Checklist: