Skip to content
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

Update of Python microservice example #2199

Merged
merged 8 commits into from
Jun 21, 2024

Conversation

eickler
Copy link
Contributor

@eickler eickler commented Jun 18, 2024

I took the liberty and rewrote the text a little. My main goals are:

  • Be a bit more gentle phrasing than the original text.
  • Be understandable and executable in the order that the text is read.
  • Illustrate production use by default (to enable the user to create microservices that are in line with our SLA requirements).
  • Do not depend on outdated version.

I'll still create PRs for the cumulocity-examples and c8y_microservice-python.

@BeateRixen Do you happen to know who the SME for reviewing this could be?

Copy link
Contributor

Preview available here

@BeateRixen
Copy link
Collaborator

I took the liberty and rewrote the text a little. My main goals are:

  • Be a bit more gentle phrasing than the original text.
  • Be understandable and executable in the order that the text is read.
  • Illustrate production use by default (to enable the user to create microservices that are in line with our SLA requirements).
  • Do not depend on outdated version.

I'll still create PRs for the cumulocity-examples and c8y_microservice-python.

@BeateRixen Do you happen to know who the SME for reviewing this could be?

I added @jochenendres and @mgrumann from the responsible team, but regarding Python I have no clue.

Added logging so that some output can be seen in the Admin app, bumped
to recent manifest version, advertised automated authentication.
@eickler
Copy link
Contributor Author

eickler commented Jun 19, 2024

Thanks ... @jochenendres @mgrumann Feel free to nominate someone else. I noticed a little cosmetic issue, not sure if that would be a one liner fix or more involved: The microservice proxies reports authentication errors differently from the normal REST endpoints. You get an "internalError" reported, which shouldn't be the case if you just leave out the credentials ...

eickler and others added 3 commits June 19, 2024 11:47
Co-authored-by: BeateRixen <90445236+BeateRixen@users.noreply.github.com>
Co-authored-by: BeateRixen <90445236+BeateRixen@users.noreply.github.com>
Co-authored-by: BeateRixen <90445236+BeateRixen@users.noreply.github.com>
@eickler eickler requested a review from BeateRixen June 19, 2024 13:25
@eickler eickler requested a review from BeateRixen June 19, 2024 13:37
@eickler eickler merged commit 76d35db into develop Jun 21, 2024
1 check passed
@eickler eickler deleted the feature/MTM-51290-python-microservice-fix branch June 21, 2024 09:41

* Is a multi-tenant microservice, which means that it runs only once even if many customers are subscribed to it.
* Has two replicas as required for highly available production microservices. Note: For development purposes where high availability is not required, you can set this to one replica only.
* Has so-called liveness and readiness probes that {{< product-c8y-iot >}} uses to check if your microservice is healthy and can run.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For my understanding: with this framework , should the customer implement the /health endpoint? If yes, maybe a hint mentioning it here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I merged the PR already before your comment. I updated the text, but the change will only go into the next PR.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No problem. I have not noticed that it has been merged :) probably it was done while I was reviewing it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants