Skip to content

Commit 7b16d97

Browse files
authored
improve developer experience (#342)
* black -> ruff format * only re-build containers when we _need_ to * migrate to modern sqlalachemy syntax/decarativebase * use updated utc spell * ignore state dir for docker buildwq * use ty for type-checking * address the basics * migrate from sqlalchemy back_refs to back_populates * ignore idiosyncracies of flask/Jinja * wrap up database type maintenance * put comment in right place * don't needlessly re-install pytest, its in uv.lock * squish warning * squash warning: migrate to new flask_security form * squash warning: move to correct gitignore pattern matching * upgrade to psycopg 3 * clear more extraneous warnings * minimize copy into docker * gunicorn.conf.py * ope, bug * do a way with "Optional" * forward refs
1 parent e08f9ec commit 7b16d97

43 files changed

Lines changed: 936 additions & 1061 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.dockerignore

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
/.env
22
/.git
33
/.ruff_cache
4-
/dev
54
/node_modules
65
__pycache__
7-
.swp
8-
buildanddeploy
6+
*.swp

.github/workflows/ci.yml

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ jobs:
1919
- uses: astral-sh/setup-uv@v5
2020
- run: uv python install 3.13
2121
- run: uv sync --frozen
22-
- run: uv run black --check --diff .
22+
- run: uv run ruff format --check --diff .
2323
- run: uv run ruff check .
2424

2525
type-check:
@@ -30,7 +30,7 @@ jobs:
3030
- uses: astral-sh/setup-uv@v5
3131
- run: uv python install 3.13
3232
- run: uv sync --frozen
33-
- run: uv run mypy --config-file pyproject.toml .
33+
- run: uv run ty check cabotage
3434

3535
security-check:
3636
runs-on: ubuntu-24.04-arm
@@ -78,12 +78,12 @@ jobs:
7878
- name: Run migrations
7979
run: uv run flask db upgrade
8080
env:
81-
CABOTAGE_SQLALCHEMY_DATABASE_URI: postgresql://postgres@localhost/cabotage_test
81+
CABOTAGE_SQLALCHEMY_DATABASE_URI: postgresql+psycopg://postgres@localhost/cabotage_test
8282
FLASK_APP: cabotage.server.wsgi
8383
- name: Run tests
8484
run: uv run pytest tests/ -v
8585
env:
86-
CABOTAGE_SQLALCHEMY_DATABASE_URI: postgresql://postgres@localhost/cabotage_test
86+
CABOTAGE_SQLALCHEMY_DATABASE_URI: postgresql+psycopg://postgres@localhost/cabotage_test
8787
CABOTAGE_CELERY_BROKER_URL: redis://localhost:6379/0
8888
CABOTAGE_CELERY_RESULTS_URL: redis://localhost:6379/0
8989
CABOTAGE_TESTING: "True"

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,3 +12,4 @@ cabotage/client/static/main.min.css
1212
cabotage/client/static/main.min.js
1313
/htmlcov
1414
/.coverage
15+
/.state

Dockerfile

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,10 +27,10 @@ COPY --from=moby/buildkit:v0.28.0-rootless /usr/bin/fuse-overlayfs /usr/bin/fuse
2727
COPY --from=moby/buildkit:v0.28.0-rootless /usr/bin/newuidmap /usr/bin/newuidmap
2828
COPY --from=moby/buildkit:v0.28.0-rootless /usr/bin/newgidmap /usr/bin/newgidmap
2929

30-
ENV PYTHONUNBUFFERED 1
31-
ENV UV_PROJECT_ENVIRONMENT=/opt/cabotage-app \
32-
UV_COMPILE_BYTECODE=1 \
33-
UV_LINK_MODE=copy
30+
ENV PYTHONUNBUFFERED=1
31+
ENV UV_PROJECT_ENVIRONMENT=/opt/cabotage-app
32+
ENV UV_COMPILE_BYTECODE=1
33+
ENV UV_LINK_MODE=copy
3434
ENV PATH="/opt/cabotage-app/bin:${PATH}"
3535

3636
COPY --from=ghcr.io/astral-sh/uv:0.11.2 /uv /usr/local/bin/uv
@@ -46,14 +46,17 @@ RUN --mount=type=cache,target=/root/.cache/uv \
4646
uv sync --locked --no-install-project --no-editable \
4747
$(if [ "$DEVEL" != "yes" ]; then echo '--no-dev'; fi)
4848

49-
COPY . /opt/cabotage-app/src/
50-
5149
# Build and minify static assets for production
5250
COPY --from=oven/bun:1-slim /usr/local/bin/bun /usr/local/bin/bun
51+
COPY package.json bun.lock /opt/cabotage-app/src/
52+
COPY cabotage/ /opt/cabotage-app/src/cabotage/
5353
RUN if [ "$DEVEL" != "yes" ]; then \
5454
cd /opt/cabotage-app/src && \
5555
bun install --frozen-lockfile && \
5656
bun run build && \
5757
rm -rf node_modules; \
5858
fi
5959

60+
COPY migrations/ /opt/cabotage-app/src/migrations/
61+
COPY gunicorn.conf.py /opt/cabotage-app/src/
62+

Makefile

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,13 @@ default:
1414
@echo
1515
@exit 1
1616

17-
start:
18-
docker-compose up --build --detach
17+
.state/docker-build-base: Dockerfile uv.lock package.json
18+
docker compose build --force-rm base
19+
mkdir -p .state
20+
touch .state/docker-build-base
21+
22+
start: .state/docker-build-base
23+
docker-compose up --detach
1924
@$(MAKE) seed
2025

2126
rebuild: start
@@ -44,24 +49,24 @@ routes:
4449
lock:
4550
uv lock
4651

47-
test:
52+
test: .state/docker-build-base
4853
docker compose exec db psql -U postgres -tc "SELECT 1 FROM pg_database WHERE datname = 'cabotage_test'" | grep -q 1 || \
4954
docker compose exec db psql -U postgres -c "CREATE DATABASE cabotage_test" && \
5055
docker compose exec db psql -U postgres -d cabotage_test -c "CREATE EXTENSION IF NOT EXISTS citext; CREATE EXTENSION IF NOT EXISTS pgcrypto;"
5156
docker compose run --rm \
52-
-e CABOTAGE_SQLALCHEMY_DATABASE_URI=postgresql://postgres@db/cabotage_test \
57+
-e CABOTAGE_SQLALCHEMY_DATABASE_URI=postgresql+psycopg://postgres@db/cabotage_test \
5358
-e CABOTAGE_TESTING=True \
5459
-e FLASK_APP=cabotage.server.wsgi \
55-
base sh -c "uv pip install pytest && python3 -m flask db upgrade && python3 -m pytest tests/ -v $(ARGS)"
60+
base sh -c "python3 -m flask db upgrade && python3 -m pytest tests/ -v $(ARGS)"
5661

57-
reformat:
58-
docker compose run --build --rm base black .
62+
reformat: .state/docker-build-base
63+
docker compose run --rm base ruff format .
5964

60-
lint:
61-
docker compose run --build --rm base bin/lint
65+
lint: .state/docker-build-base
66+
docker compose run --rm base bin/lint
6267

63-
security-check:
64-
docker compose run --build --rm base bandit -c pyproject.toml -r .
68+
security-check: .state/docker-build-base
69+
docker compose run --rm base bandit -c pyproject.toml -r .
6570

66-
type-check:
67-
docker compose run --build --rm base mypy --config-file pyproject.toml .
71+
type-check: .state/docker-build-base
72+
docker compose run --rm base ty check cabotage

Procfile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
web: gunicorn -c gunicorn.conf -b unix:/var/run/cabotage/cabotage.sock -w "4" --threads "100" cabotage.server.wsgi:app
1+
web: gunicorn -c gunicorn.conf.py -b unix:/var/run/cabotage/cabotage.sock -w "4" --threads "100" cabotage.server.wsgi:app
22
worker: celery -A cabotage.celery.worker.celery_app worker -l info -E
33
worker-beat: celery -A cabotage.celery.worker.celery_app beat -l info -S redbeat.RedBeatScheduler
44
release: python -m flask db upgrade head

bin/lint

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,5 +2,5 @@
22

33
set -ex
44

5-
black --check --diff .
5+
ruff format --check --diff .
66
ruff check .

cabotage/celery/tasks/branch_deploy.py

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ def _create_app_env_for_branch_deploy(
166166
activity = Activity(
167167
verb="create",
168168
object=app_env,
169-
data={"timestamp": datetime.datetime.utcnow().isoformat()},
169+
data={"timestamp": datetime.datetime.now(datetime.timezone.utc).isoformat()},
170170
)
171171
db.session.add(activity)
172172
return app_env
@@ -399,7 +399,9 @@ def _build_images_for_app_envs(app_envs, commit_sha, installation_id):
399399
activity = Activity(
400400
verb="submit",
401401
object=image,
402-
data={"timestamp": datetime.datetime.utcnow().isoformat()},
402+
data={
403+
"timestamp": datetime.datetime.now(datetime.timezone.utc).isoformat()
404+
},
403405
)
404406
db.session.add(activity)
405407
images.append(image)
@@ -509,7 +511,7 @@ def _render_pr_comment_body(environment):
509511
base_url = f"{scheme}://{server}"
510512

511513
lines = [
512-
f"**Branch Deploy** for `{environment.slug}` " f"in **{project.name}**",
514+
f"**Branch Deploy** for `{environment.slug}` in **{project.name}**",
513515
"",
514516
"| Service | Status | Preview | Updated (UTC) |",
515517
"| :--- | :--- | :--- | :--- |",
@@ -689,7 +691,7 @@ def create_branch_deploy(project, pr_number, head_sha, installation_id, head_ref
689691
activity = Activity(
690692
verb="create",
691693
object=environment,
692-
data={"timestamp": datetime.datetime.utcnow().isoformat()},
694+
data={"timestamp": datetime.datetime.now(datetime.timezone.utc).isoformat()},
693695
)
694696
db.session.add(activity)
695697

cabotage/celery/tasks/build.py

Lines changed: 22 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,8 @@ def _fetch_github_access_token(application):
175175
or application.github_app_installation_id
176176
):
177177
try:
178+
if github_app.app_id is None:
179+
raise BuildError("GitHub App ID not configured")
178180
auth = GithubAppAuth(github_app.app_id, github_app.app_private_key_pem)
179181
gi = GithubIntegration(auth=auth)
180182
access_token = gi.get_access_token(
@@ -271,16 +273,16 @@ def file_path(filename):
271273
os.chdir(tempdir)
272274
try:
273275
dockerfile_object.content = dockerfile_body
276+
dockerfile_env_vars = list(dockerfile_object.envs.keys())
274277
finally:
275278
os.chdir(previous_dir)
276-
dockerfile_env_vars = list(dockerfile_object.envs.keys())
277279
try:
278280
processes = procfile.loads(procfile_body)
279281
except ValueError as exc:
280282
raise BuildError(f"error parsing Procfile: {exc}")
281283

282284
for process_name, process_def in processes.items():
283-
if re.search("\s", process_name) is not None:
285+
if re.search(r"\s", process_name) is not None:
284286
raise BuildError(
285287
f'Invalid process name: "{process_name}" in Procfile, '
286288
"may not contain whitespace."
@@ -613,9 +615,7 @@ def build_release_buildkit(release):
613615
"done\n"
614616
f'buildctl --addr={sock_addr} "$@"\n'
615617
)
616-
os.chmod(
617-
wrapper, 0o755
618-
) # nosec B103 — wrapper script must be executable
618+
os.chmod(wrapper, 0o755) # nosec B103 — wrapper script must be executable
619619
buildctl_command = [wrapper]
620620

621621
try:
@@ -669,6 +669,10 @@ def _fetch_github_file(
669669
g = Github(access_token)
670670
try:
671671
content_file = g.get_repo(github_repository).get_contents(filename, ref=ref)
672+
if isinstance(content_file, list):
673+
raise BuildError(
674+
f"Expected a file but got a directory listing for {filename}"
675+
)
672676
if content_file.encoding == "base64":
673677
return b64decode(content_file.content).decode()
674678
return content_file.content
@@ -773,7 +777,7 @@ def fetch_image_build_cache_volume_claim(core_api_instance, buildable):
773777
return volume_claim
774778

775779

776-
def build_image_buildkit(image=None):
780+
def build_image_buildkit(image: Image):
777781
bke = BuildkitEnv(image.repository_name)
778782
registry = bke.registry
779783
buildkit_image = bke.buildkit_image
@@ -1096,9 +1100,7 @@ def build_image_buildkit(image=None):
10961100
"done\n"
10971101
f'buildctl --addr={sock_addr} "$@"\n'
10981102
)
1099-
os.chmod(
1100-
wrapper, 0o755
1101-
) # nosec B103 — wrapper script must be executable
1103+
os.chmod(wrapper, 0o755) # nosec B103 — wrapper script must be executable
11021104
buildctl_command = [wrapper]
11031105

11041106
try:
@@ -1577,7 +1579,7 @@ def build_omnibus_buildkit(image, release):
15771579

15781580

15791581
@shared_task()
1580-
def run_image_build(image_id=None, buildkit=False):
1582+
def run_image_build(image_id: str, buildkit: bool = False):
15811583
from cabotage.utils.config_templates import TemplateResolutionError
15821584

15831585
current_app.config["REGISTRY_AUTH_SECRET"]
@@ -1733,7 +1735,7 @@ def run_image_build(image_id=None, buildkit=False):
17331735
"user_id": "automation",
17341736
"deployment_id": image.image_metadata.get("id", None),
17351737
"description": image.image_metadata.get("description", None),
1736-
"timestamp": datetime.datetime.utcnow().isoformat(),
1738+
"timestamp": datetime.datetime.now(datetime.timezone.utc).isoformat(),
17371739
},
17381740
)
17391741
db.session.add(activity)
@@ -1749,7 +1751,7 @@ def run_image_build(image_id=None, buildkit=False):
17491751

17501752

17511753
@shared_task()
1752-
def run_release_build(release_id=None):
1754+
def run_release_build(release_id: str):
17531755
from cabotage.utils.config_templates import TemplateResolutionError
17541756

17551757
release = None
@@ -1897,7 +1899,9 @@ def run_release_build(release_id=None):
18971899
"user_id": "automation",
18981900
"deployment_id": release.release_metadata.get("id", None),
18991901
"description": release.release_metadata.get("description", None),
1900-
"timestamp": datetime.datetime.utcnow().isoformat(),
1902+
"timestamp": datetime.datetime.now(
1903+
datetime.timezone.utc
1904+
).isoformat(),
19011905
},
19021906
)
19031907
db.session.add(activity)
@@ -1932,7 +1936,7 @@ def run_release_build(release_id=None):
19321936

19331937

19341938
@shared_task()
1935-
def run_omnibus_build(image_id=None):
1939+
def run_omnibus_build(image_id: str):
19361940
"""Build image + release in a single K8s Job for auto-deploys.
19371941
19381942
Avoids mounting the build cache volume twice by combining both build
@@ -2028,7 +2032,9 @@ def run_omnibus_build(image_id=None):
20282032
"user_id": "automation",
20292033
"deployment_id": image.image_metadata.get("id", None),
20302034
"description": image.image_metadata.get("description", None),
2031-
"timestamp": datetime.datetime.utcnow().isoformat(),
2035+
"timestamp": datetime.datetime.now(
2036+
datetime.timezone.utc
2037+
).isoformat(),
20322038
},
20332039
)
20342040
db.session.add(activity)
@@ -2140,7 +2146,7 @@ def run_omnibus_build(image_id=None):
21402146
"user_id": "automation",
21412147
"deployment_id": image.image_metadata.get("id", None),
21422148
"description": image.image_metadata.get("description", None),
2143-
"timestamp": datetime.datetime.utcnow().isoformat(),
2149+
"timestamp": datetime.datetime.now(datetime.timezone.utc).isoformat(),
21442150
},
21452151
)
21462152
db.session.add(activity)

cabotage/celery/tasks/deploy.py

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1047,7 +1047,7 @@ def fetch_ingress(networking_api, release, ingress):
10471047
if exc.status == 404:
10481048
return create_ingress(networking_api, release, ingress)
10491049
raise DeployError(
1050-
"Unexpected exception fetching Ingress/" f"{k8s_name} in {namespace}: {exc}"
1050+
f"Unexpected exception fetching Ingress/{k8s_name} in {namespace}: {exc}"
10511051
)
10521052

10531053

@@ -1535,7 +1535,7 @@ def render_process_container(
15351535
pod_class = pod_classes[process_pod_cls]
15361536
return kubernetes.client.V1Container(
15371537
name=process_name,
1538-
image=f'{current_app.config["REGISTRY_PULL"]}/{release.repository_name}:release-{release.version}',
1538+
image=f"{current_app.config['REGISTRY_PULL']}/{release.repository_name}:release-{release.version}",
15391539
image_pull_policy="Always",
15401540
env=[
15411541
kubernetes.client.V1EnvVar(
@@ -1604,10 +1604,12 @@ def render_datadog_container(dd_api_key, datadog_tags):
16041604
kubernetes.client.V1EnvVar(name="DD_APM_ENABLED", value="true"),
16051605
kubernetes.client.V1EnvVar(name="DD_LOGS_ENABLED", value="false"),
16061606
kubernetes.client.V1EnvVar(
1607-
name="DD_CONFD_PATH", value="/tmp/null" # nosec
1607+
name="DD_CONFD_PATH",
1608+
value="/tmp/null", # nosec
16081609
),
16091610
kubernetes.client.V1EnvVar(
1610-
name="DD_AUTOCONF_TEMPLATE_DIR", value="/tmp/null" # nosec
1611+
name="DD_AUTOCONF_TEMPLATE_DIR",
1612+
value="/tmp/null", # nosec
16111613
),
16121614
kubernetes.client.V1EnvVar(name="DD_ENABLE_GOHAI", value="false"),
16131615
kubernetes.client.V1EnvVar(
@@ -2063,9 +2065,9 @@ def _get_job_schedule(process_def):
20632065
def _history_limit_for_schedule(schedule, hours=12):
20642066
"""Estimate how many times a cron schedule fires in the given window."""
20652067
from croniter import croniter
2066-
from datetime import datetime, timedelta
2068+
from datetime import datetime, timedelta, timezone
20672069

2068-
now = datetime.utcnow()
2070+
now = datetime.now(timezone.utc)
20692071
end = now + timedelta(hours=hours)
20702072
it = croniter(schedule, now)
20712073
count = 0

0 commit comments

Comments
 (0)