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

Migrate Flask tutorial to Charmcraft RTD #1976

Open
wants to merge 63 commits into
base: main
Choose a base branch
from

Conversation

erinecon
Copy link

The current Flask tutorial: https://juju.is/docs/sdk/write-your-first-kubernetes-charm-for-a-flask-app

This PR will move the tutorial off of Discourse and Charm SDK and onto the Charmcraft RTD.

@erinecon erinecon marked this pull request as ready for review November 21, 2024 19:47

.. code-block:: bash

sudo snap install charmcraft --channel latest/edge --classic
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we use latest/edge here or switch to stable? Rockcraft above is using stable

Copy link
Author

Choose a reason for hiding this comment

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

Does the stable version of charmcraft support all the extensions? For Flask it does, but since this is under reuse I wanted to make sure the setup was as extension-agnostic as possible

docs/tutorial/code/flask/task.yaml Outdated Show resolved Hide resolved
# [docs:deploy-juju-model-end]

# [docs:deploy-nginx]
juju deploy nginx-ingress-integrator --channel=latest/edge --revision 122
Copy link
Contributor

Choose a reason for hiding this comment

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

Any particular reason for pinning the revision please? I think lates/edge should get the latest revision

Copy link
Author

@erinecon erinecon Nov 22, 2024

Choose a reason for hiding this comment

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

latest/edge did not get the latest revision in the GitHub CI test -- it did for my local testing though.

In GitHub, latest/edge grabbed revision 44 (suspiciously, this is the latest revision for the Ubuntu 22.04 base while 122 is the latest revision for the Ubuntu 20.04 base) and revision 44 ran into hook failed: "config-changed" errors upon deployment

One other thing to note: When I specified the base instead of revision, the "hook failed" issues went away.

docs/tutorial/code/flask/task.yaml Outdated Show resolved Hide resolved
docs/tutorial/code/flask/task.yaml Outdated Show resolved Hide resolved
docs/tutorial/code/flask/task.yaml Outdated Show resolved Hide resolved
docs/tutorial/code/flask/task.yaml Outdated Show resolved Hide resolved
docs/tutorial/code/flask/task.yaml Outdated Show resolved Hide resolved
docs/tutorial/code/flask/task.yaml Outdated Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

When I go to the readthedocs for this PR (you can do that by clicking as indicated in the screenshot on the PR page), I can't see the tutorial being rendered? It is usually better to review it there to check for that the correct lines were included etc
Screenshot 2024-11-23 at 1 41 38 am

Copy link
Author

Choose a reason for hiding this comment

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

I see the tutorial being rendered, but there isn't a dedicated tutorial section. I can reorganise

Copy link
Author

Choose a reason for hiding this comment

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

I added a tutorial section, and the Flask tutorial shows up underneath it when I check the rendered page. Let me know if you don't see it

@jdkandersson
Copy link
Contributor

Thanks for doing this, I know it isn't easy especially because this involved code 🙂

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.

2 participants