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

extensions: new flask extension #317

Merged
merged 42 commits into from
Sep 28, 2023
Merged
Show file tree
Hide file tree
Changes from 29 commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
3f3a222
Add flask framework extension
weiiwang01 Jul 4, 2023
caa7dbf
Update documents in the flask extension
weiiwang01 Jul 5, 2023
128c58d
Fix some linting issues
weiiwang01 Jul 5, 2023
016b2b1
Fix some linting issues
weiiwang01 Jul 5, 2023
10ec7d4
Update the name of the extension
weiiwang01 Jul 5, 2023
8f622fa
Some document and naming changes
weiiwang01 Jul 10, 2023
d5a048d
Add a spread test for the flask extension
weiiwang01 Jul 17, 2023
dde4b33
raise an error when requirements.txt doesn't exist
weiiwang01 Jul 17, 2023
f40b7dd
Fix the linting issue
weiiwang01 Jul 17, 2023
2364f45
Merge branch 'main' into main
weiiwang01 Jul 17, 2023
ce8c2b5
Merge branch 'main' into main
weiiwang01 Jul 21, 2023
2f4e560
Add more items to the flask extension spread test
weiiwang01 Jul 24, 2023
d6926e3
Merge branch 'main' into main
sergiusens Jul 26, 2023
21d7dfb
Add support for 20.04 in flask extension
weiiwang01 Jul 26, 2023
2bc660d
Merge branch 'main' into main
sergiusens Jul 26, 2023
310408f
Add extensions section in the rockcraft.yaml doc
weiiwang01 Jul 27, 2023
6f1a627
Update document and part merging
weiiwang01 Aug 10, 2023
8c25807
Only allow to overwrite prime in flask/install-app
weiiwang01 Aug 10, 2023
1e8f860
Update part merging and documents
weiiwang01 Aug 13, 2023
2830f97
Use root snippet to modify parts
weiiwang01 Aug 14, 2023
d5890cd
Fix the spread test for the flask extension
weiiwang01 Aug 14, 2023
efbfa3b
Add services in flask extension and fix documents
weiiwang01 Aug 22, 2023
781eab6
Add tmp in bare containers and fix docs
weiiwang01 Aug 22, 2023
97aa7e2
Fix some linting problems
weiiwang01 Aug 22, 2023
b31ba69
Merge branch 'main' into main
weiiwang01 Aug 22, 2023
75049de
Change to British English and update wordlist
weiiwang01 Aug 22, 2023
10ec23d
Add some unit tests for flask extension
weiiwang01 Aug 28, 2023
3212eae
Fix the flask extension on windows
weiiwang01 Aug 28, 2023
0fe5723
Merge branch 'main' into main
weiiwang01 Aug 28, 2023
ce2ce3e
Apply suggestions from code review
weiiwang01 Aug 31, 2023
6e3f682
Apply suggestions from code review
weiiwang01 Aug 31, 2023
463e298
Update documents
weiiwang01 Aug 31, 2023
cab47e8
Mandate the prime in flask/install-app
weiiwang01 Sep 1, 2023
b89ceb7
Add flask extension service overwrite test
weiiwang01 Sep 1, 2023
bf684a3
Fix linting and spread
weiiwang01 Sep 1, 2023
ce9e067
Fix the shellcheck linting problem
weiiwang01 Sep 4, 2023
3841e78
Apply suggestions from reviews
weiiwang01 Sep 21, 2023
2340cb6
Update the flask extension design
weiiwang01 Sep 25, 2023
5487cb5
Fix some linting issues
weiiwang01 Sep 25, 2023
1df6a35
Update test_flask_framework.py
weiiwang01 Sep 25, 2023
5d3a099
Update flask_framework.py
weiiwang01 Sep 27, 2023
c51286e
Remove working-dir from flask service
weiiwang01 Sep 28, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/.wordlist.txt
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ fs
GID
github
GPG
Gunicorn
https
init
interoperable
Expand Down
1 change: 1 addition & 0 deletions docs/how-to/index.rst
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,4 @@ adapt the steps to fit your specific requirements.
Convert an entrypoint to a Pebble layer <convert-to-pebble-layer.rst>
contribute-docs
use-chisel
Use flask extension <use-flask-extension.rst>
weiiwang01 marked this conversation as resolved.
Show resolved Hide resolved
49 changes: 49 additions & 0 deletions docs/how-to/use-flask-extension.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
Using the flask extension
-------------------------

The Flask extension is compatible with ``bare``, ``ubuntu:20.04``, and
weiiwang01 marked this conversation as resolved.
Show resolved Hide resolved
``ubuntu:22.04``. To employ it, include ``extensions: [flask]`` in your
weiiwang01 marked this conversation as resolved.
Show resolved Hide resolved
``rockcraft.yaml`` file.

Example:

.. code-block:: yaml
Copy link
Contributor

Choose a reason for hiding this comment

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

Check that this meets all the requirements, e.g., we have made the prime required now

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Documents has been updated, thanks!


name: example
weiiwang01 marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we can improve the fields a bit, even if they are just an example:

name: example-flask
summary: A Flask application
description: A ROCK packing a Flask application via the flask extension

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, indeed, updated, thanks!

summary: Example.
description: Example.
version: "0.1"
base: bare
license: Apache-2.0

extensions:
- flask

Managing project files with the flask extension
-----------------------------------------------

By default, all files within the Flask project directory are copied, excluding
Copy link
Collaborator

Choose a reason for hiding this comment

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

I thought we had agreed to change this default behavior, so that the user must always declare the files to copy (for security reasons). But I admit my memory may be wrong 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.

Yes, maybe we can confirm that in today's meeting.

certain common files and directories, such as ``node_modules``. However,
this behaviour can be tailored to either specifically include or exclude files
from the Flask project directory in the rock image.
weiiwang01 marked this conversation as resolved.
Show resolved Hide resolved

To include only select files from the project directory in the rock image,
weiiwang01 marked this conversation as resolved.
Show resolved Hide resolved
append the following part to ``rockcraft.yaml``:
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think the example is good but we should explain the prime bit just a bit to give some necessary context. Something like:

You can include and exclude files from the project directory in the ROCK image by using the standard prime declaration on the specially-named flask/install-app part. For example, to include only select files:

(and then)

or to exclude specific files:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that's better, updated, thanks!


.. code-block:: yaml

flask/install-app:
prime:
- srv/flask/app/static
- srv/flask/app/.env
- srv/flask/app/webapp
- srv/flask/app/templates

To exclude certain files from the project directory in the rock image,
add the following part to ``rockcraft.yaml``:

.. code-block:: yaml

flask/install-app:
prime:
- -srv/flask/app/charmcraft.auth
16 changes: 16 additions & 0 deletions docs/reference/extensions.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
Extensions
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍 ty for starting this page

**********

Just as the snapcraft extensions are designed to simplify snap creation,
weiiwang01 marked this conversation as resolved.
Show resolved Hide resolved
Rockcraft extensions are crafted to expand and modify the user-provided
rockcraft manifest file, aiming to minimise the boilerplate code when
Copy link
Collaborator

Choose a reason for hiding this comment

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

hmm not sure we should use the word "manifest" here, I don't think we use it anywhere else in the docs. "project" is probably more appropriate.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Gotcha, updated, thanks!

initiating a new rock.

The flask extension
weiiwang01 marked this conversation as resolved.
Show resolved Hide resolved
-------------------

The Flask extension streamlines the process of building Flask application rocks.

It facilitates the installation of Flask application dependencies, including
Gunicorn, in the rock image. Additionally, it transfers your project files to
``/srv/flask/app`` within the rock image.
1 change: 1 addition & 0 deletions docs/reference/index.rst
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,4 @@ Rockcraft's components, commands and keywords.
parts
commands
part_properties
extensions
12 changes: 12 additions & 0 deletions docs/reference/rockcraft.yaml.rst
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,18 @@ The set of parts that compose the ROCK's contents
Rockcraft. All ROCKs have Pebble as their entrypoint, and thus you must use
``services`` to define your container application.

``extensions``
--------------

**Type**: list[string]

**Required**: No

Extensions to apply to this rock image.
weiiwang01 marked this conversation as resolved.
Show resolved Hide resolved

Currently supported extensions:

- ``flask`` (experimental)

Example
=======
Expand Down
4 changes: 4 additions & 0 deletions rockcraft/extensions/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,7 @@
"register",
"unregister",
]

from .flask import Flask as _Flask
Copy link
Collaborator

Choose a reason for hiding this comment

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

why the rename?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not exactly sure why, but indeed it's unnecessary, updated, thanks!


register("flask", _Flask)
169 changes: 169 additions & 0 deletions rockcraft/extensions/flask.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,169 @@
# -*- Mode:Python; indent-tabs-mode:nil; tab-width:4 -*-
#
# Copyright 2023 Canonical Ltd.
#
# This program is free software: you can redistribute it and/or modify
# it under the terms of the GNU General Public License version 3 as
# published by the Free Software Foundation.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.

"""An experimental extension for the Flask framework."""

import copy
import os
import posixpath
from typing import Any, Dict, Optional, Tuple
Copy link
Contributor

Choose a reason for hiding this comment

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

Which python version is used on rockcraft? Some of these are deprecated and should be replaced with the built in types dict, tuple etc

Copy link
Contributor Author

Choose a reason for hiding this comment

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

rockcraft is targeting python>=3.8

Copy link
Collaborator

Choose a reason for hiding this comment

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

@jdkandersson is right with regards to the current best practices but the rest of rockcraft's codebase is still using the types from typing, so I suggest we keep this for consistency and then in a separate issue we can convert all code


from overrides import override

from ._utils import _apply_extension_property
from .extension import Extension


class Flask(Extension):
"""An extension for constructing Python applications based on the Flask framework."""

@staticmethod
@override
def get_supported_bases() -> Tuple[str, ...]:
"""Return supported bases."""
return "bare", "ubuntu:20.04", "ubuntu:22.04"

@staticmethod
@override
def is_experimental(base: Optional[str]) -> bool:
"""Check if the extension is in an experimental state."""
return True

@override
def get_root_snippet(self) -> Dict[str, Any]:
"""Fill in some default root components for Flask.

Default values:
- run_user: _daemon_
- build-base: ubuntu:22.04 (only if user specify bare without a build-base)
- platform: amd64
"""
snippet: Dict[str, Any] = {}
if "run_user" not in self.yaml_data:
snippet["run_user"] = "_daemon_"
if (
"build-base" not in self.yaml_data
and self.yaml_data.get("base", "bare") == "bare"
):
snippet["build-base"] = "ubuntu:22.04"
if "platforms" not in self.yaml_data:
snippet["platforms"] = {"amd64": {}}
current_parts = copy.deepcopy(self.yaml_data.get("parts", {}))
current_parts.update(self._gen_new_parts())
snippet["parts"] = current_parts
snippet["services"] = self._gen_services()
return snippet

def _gen_services(self):
"""Return the services snipped to be applied to the rockcraft file."""
services = {
"flask": {
"override": "replace",
"startup": "enabled",
"command": "/bin/python3 -m gunicorn --bind 0.0.0.0:8000 app:app",
"user": "_daemon_",
"working-dir": "/srv/flask/app",
}
}
existing_services = self.yaml_data.get("services", {})
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think for consistency with the part handling this should also be a deepcopy of the existing data

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added, perhaps we should define the yaml_data as a property method. This way, we can prevent such issues at the framework level.

@property
def yaml_data(self) -> Dict[str, Any]:
    return self._yaml_data

for existing_service_name, existing_service in existing_services.items():
if existing_service_name in services:
services[existing_service_name].update(existing_service)
else:
services[existing_service_name] = existing_service
return services

@override
def get_part_snippet(self) -> Dict[str, Any]:
"""Return the part snippet to apply to existing parts."""
return {}

def _merge_part(self, base_part: dict, new_part: dict) -> dict:
"""Merge two part definitions by the extension part merging rule."""
result = {}
properties = set(base_part.keys()).union(set(new_part.keys()))
for property_name in properties:
if property_name in base_part and property_name not in new_part:
result[property_name] = base_part[property_name]
elif property_name not in base_part and property_name in new_part:
result[property_name] = new_part[property_name]
else:
result[property_name] = _apply_extension_property(
base_part[property_name], new_part[property_name]
)
return result

def _merge_existing_part(self, part_name: str, part_def: dict) -> dict:
"""Merge the new part with the existing part in the current rockcraft.yaml."""
existing_part = self.yaml_data.get("parts", {}).get(part_name, {})
return self._merge_part(existing_part, part_def)

def _gen_new_parts(self) -> Dict[str, Any]:
"""Generate new parts for the flask extension.

Parts added:
- flask/dependencies: install Python dependencies
- flask/install-app: copy the flask project into the OCI image
"""
if not (self.project_root / "requirements.txt").exists():
raise ValueError(
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think errors.ExtensionError is more specific 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.

Indeed, updated, thanks!

"missing requirements.txt file, "
"flask extension requires this file with flask specified as a dependency"
)
ignores = [".git", "node_modules", ".yarn"]
source_files = [
f
for f in os.listdir(self.project_root)
Copy link
Contributor

Choose a reason for hiding this comment

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

what about using pathlib?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated, thanks!

if f not in ignores and not f.endswith(".rock")
]
renaming_map = {f: posixpath.join("srv/flask/app", f) for f in source_files}
Copy link
Contributor

Choose a reason for hiding this comment

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

I was trying to find the handling of requiring prime and checking that everything in there has the srv/flask/app prefix?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That has been added in the latest commit, thanks!

install_app_part_name = "flask/install-app"
dependencies_part_name = "flask/dependencies"
# Users are required to compile any static assets prior to executing the
# rockcraft pack command, so assets can be included in the final OCI image.
install_app_part = {
"plugin": "dump",
"source": ".",
"organize": renaming_map,
"stage": list(renaming_map.values()),
}
dependencies_part = {
"plugin": "python",
"stage-packages": ["python3-venv"],
"source": ".",
"python-packages": ["gunicorn"],
"python-requirements": ["requirements.txt"],
}
snippet = {
dependencies_part_name: self._merge_existing_part(
dependencies_part_name, dependencies_part
),
install_app_part_name: self._merge_existing_part(
install_app_part_name, install_app_part
),
}
if self.yaml_data["base"] == "bare":
snippet["flask/container-processing"] = {
"plugin": "nil",
"source": ".",
"override-prime": "craftctl default\nmkdir -p tmp\nchmod 777 tmp",
Copy link
Collaborator

Choose a reason for hiding this comment

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

a bit surprised that this works, I guess craftctl default is changing the current dir to CRAFT_PRIME?
regardless, I think it's cleaner/more idiomatic to do something like "override-build": "mkdir -m 777 ${CRAFT_PART_INSTALL}/tmp"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's more cleaner, thanks, updated!

}
return snippet

@override
def get_parts_snippet(self) -> Dict[str, Any]:
"""Create necessary parts to facilitate the flask application."""
Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess this docstring is redundant/misleading

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I forgot to updated the docstring, updated, thanks!

return {}
7 changes: 6 additions & 1 deletion rockcraft/providers.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,12 @@ def get_command_environment() -> Dict[str, Optional[str]]:
env["ROCKCRAFT_MANAGED_MODE"] = "1"

# Pass-through host environment that target may need.
for env_key in ["http_proxy", "https_proxy", "no_proxy"]:
for env_key in [
"http_proxy",
"https_proxy",
"no_proxy",
"ROCKCRAFT_ENABLE_EXPERIMENTAL_EXTENSIONS",
]:
if env_key in os.environ:
env[env_key] = os.environ[env_key]

Expand Down
1 change: 1 addition & 0 deletions tests/spread/general/extension-flask/README
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
test
8 changes: 8 additions & 0 deletions tests/spread/general/extension-flask/app.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
from flask import Flask # pyright: ignore[reportMissingImports]

app = Flask(__name__)


@app.route("/")
def hello_world():
return "Hello, World!"
Empty file.
1 change: 1 addition & 0 deletions tests/spread/general/extension-flask/requirements.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
flask
14 changes: 14 additions & 0 deletions tests/spread/general/extension-flask/rockcraft.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
name: flask-extension
summary: OCI image for a flask project.
description: OCI image for a flask project.
version: "0.1"
base: bare
license: Apache-2.0

extensions:
weiiwang01 marked this conversation as resolved.
Show resolved Hide resolved
- flask

parts:
flask/install-app:
prime:
- -srv/flask/app/README
1 change: 1 addition & 0 deletions tests/spread/general/extension-flask/static/js/test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
console.log("hello")
31 changes: 31 additions & 0 deletions tests/spread/general/extension-flask/task.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
summary: flask extension test

execute: |
export ROCKCRAFT_ENABLE_EXPERIMENTAL_EXTENSIONS=true

run_rockcraft pack

test -f flask-extension_0.1_amd64.rock
test ! -d work

# Ensure docker does not have this container image
docker rmi --force flask-extension
# Install container
sudo /snap/rockcraft/current/bin/skopeo --insecure-policy copy oci-archive:flask-extension_0.1_amd64.rock docker-daemon:flask-extension:latest
# Ensure container exists
docker images flask-extension | MATCH "flask-extension"

# ensure container doesn't exist
docker rm -f flask-extension-container

# test the flask project is ready to run inside the container
tigarmo marked this conversation as resolved.
Show resolved Hide resolved
docker run --rm --entrypoint /bin/python3 flask-extension -m gunicorn --chdir /srv/flask/app --check-config app:app
docker run --rm --entrypoint /bin/python3 flask-extension -c "import pathlib;assert pathlib.Path('/srv/flask/app/static/js/test.js').is_file()"
docker run --rm --entrypoint /bin/python3 flask-extension -c "import pathlib;assert not pathlib.Path('/srv/flask/app/node_modules').exists()"

# test the part merging
docker run --rm --entrypoint /bin/python3 flask-extension -c "import pathlib;assert not pathlib.Path('/srv/flask/app/README').exists()"

restore: |
rm -f flask-extension_0.1_amd64.rock
docker rmi -f flask-extension
Loading