Skip S3 deployment on branch builds by default by zaneselvans · Pull Request #5558 · catalyst-cooperative/pudl

zaneselvans · 2026-09-05T21:49:49Z

Overview

What problem does this address?

Branch builds (any build-pudl run kicked off via workflow_dispatch from the GitHub website or API) currently test the data deployment process by running deploy-pudl, which uploads all ETL outputs to both GCS and AWS S3, to a staging/ prefix that is deleted again almost immediately.

Uploading to S3 incurs egress fees that, for a full PUDL build, cost more than running the entire ETL. We pay this on every branch build even though the nightly build already exercises the real S3 deployment every night, and the GCS deployment (no egress fees) covers nearly all of the same code paths for free.

We deploy about 30 GB of data for every build now, so that's about $3.60 per copy that gets uploaded to S3. If we push to both nightly/ and eel-hole/ paths separately, that's $7.20 per branch build (on top of the ~$1 for the ETL itself)

What did you change?

Branch builds now deploy to GCS but not S3 by default. Nightly and stable deployments are unchanged and still deploy to both.

DeploymentPlan (src/pudl/deploy/pudl.py) gains deploy_to_gcs / deploy_to_s3 overrides and symmetric upload_to_gcs / upload_to_s3 properties: GCS defaults on for every deploy type; S3 defaults on for nightly/stable and off for branch builds. A plan that would deploy nowhere is rejected.
upload_outputs() only constructs the gcsfs / s3fs clients and upload targets for the enabled destinations; _assert_permanent_paths_are_empty tolerates a missing filesystem.
pudl_deploy grows --deploy-gcs/--no-deploy-gcs and --deploy-s3/--no-deploy-s3 flags, threaded through resolve_build.
deploy-pudl.yml and build-pudl.yml expose matching workflow_dispatch inputs (deploy_to_gcs default true, deploy_to_s3 default false for build-pudl). build-pudl passes the resolved values to the batch job, and pudl_batch.sh forwards them to deploy-pudl and gates both trigger_deployment call sites — when neither target is enabled, deploy-pudl is not triggered at all (e.g. a build run only to regenerate row counts).

Closes #5557

Documentation

Make sure to update relevant aspects of the documentation:

Update docs/release_notes.rst
Review and update any other aspects of the documentation that might be affected by this PR.

Testing

New unit tests in tests/unit/deploy/deploy_pudl_test.py cover the default/override logic on DeploymentPlan and that upload_outputs skips S3 (and never constructs an S3 client) when disabled.
Try running a branch deployment and check whether it actually avoids deploying to S3 staging but does deploy to GCS staging.

To-do list

Run pixi run pytest-unit and pixi run pytest-integration (2-5 minutes total) and fix any issues that come up.
Run pixi run prek-run to run linters and static code analysis checks.
Try running a branch build with GCS but no S3 deployment.
Try running a branch build with no deployment at all.
Self-review of the PR
When you think the PR is done, run pixi run pytest-ci

krivard

This is marked as draft but actually looks pretty good as-is? some minor typos & rewords for clarity, and we should take some time at HUDL to show folks the new ropes.

You may also wish to update the following dev docs that mention branch builds:

Data Validation Reference
Nightly Data Builds

Happy to re-review any new docs content if needed.

krivard · 2026-09-08T17:14:14Z

+    inputs:
+      deploy_to_gcs:
+        type: boolean
+        description: "Deploy build outputs to GCS? (branch builds only)"
+        required: false
+        default: true
+      deploy_to_s3:
+        type: boolean
+        description: "Deploy build outputs to S3? (branch builds only; S3 egress fees are large)"
+        required: false
+        default: false


I don't love having params that are always requested but sometimes used and sometimes ignored. Alas I don't think there's a better way to do it. Once this merges I would recommend reviewing the new build interface at a HUDL so everyone knows what these mean and why the defaults are set the way they are.

krivard · 2026-09-08T17:15:15Z

+  # Nightly and stable builds always deploy to both targets; branch builds
+  # (workflow_dispatch) override these from the workflow inputs below.


krivard · 2026-09-08T17:15:52Z

+                echo "DEPLOY_TO_GCS=${{ inputs.deploy_to_gcs }}"
+                echo "DEPLOY_TO_S3=${{ inputs.deploy_to_s3 }}"
+                echo "GIT_TAG=$BUILD_ID_RAW"
+              } >> "$GITHUB_ENV"


nice use of {}!

Lol forced by the linter!

krivard · 2026-09-08T17:20:29Z


    git_tag: str
    environment: Literal["staging", "production"]
+    # Tri-state overrides for the cloud storage upload targets. ``None`` means "use


"Tri-state" always autocompletes to "area" in my brain so this massively threw me for a bit

checking understanding: the three states are (boolean True) (boolean False) (None)

Ah yeah sorry, the 3 states are True, False, and null Agree this isn't clear.

krivard · 2026-09-08T17:32:45Z

+def test_deployment_plan_rejects_no_upload_target():
+    """A plan that uploads nowhere is rejected at construction time."""


Do we need both this test and test_upload_outputs_raises_when_no_target_enabled?

Meh, maybe not. I guess they're testing the same underlying check, but entering the execution at different points. There's a little bit of plumbing between them, but not much.

zaneselvans · 2026-09-08T18:07:59Z

@krivard I think the code is pretty much ready. I just hadn't given it a full self-review to get it ready for someone else's eyeballs. But it's also a very manageable size and if you think it's good enough with these comments addressed, happy to merge!

Uploading ETL outputs to S3 incurs egress fees that, for a full PUDL build, cost more than running the entire ETL. Branch builds currently deploy to both GCS and S3 purely as a test, then delete the staged data again. The nightly build exercises the real S3 deployment every night, so re-testing it on every branch build has little marginal value, while the GCS deployment is free and covers nearly all the same code paths. Branch builds now deploy to GCS but not S3 by default. The build-pudl and deploy-pudl workflow_dispatch forms expose deploy_to_gcs / deploy_to_s3 checkboxes to override this per run, and when neither target is enabled build-pudl skips triggering deploy-pudl entirely (e.g. a build run only to regenerate row counts). Nightly and stable deployments are unchanged and still deploy to both. - DeploymentPlan gains tri-state deploy_to_gcs / deploy_to_s3 overrides and symmetric upload_to_gcs / upload_to_s3 properties: GCS defaults on for every deploy type, S3 defaults on for nightly/stable and off for branch builds. A plan that would deploy nowhere is rejected. - upload_outputs() only builds the fs clients and upload targets for the enabled destinations; _assert_permanent_paths_are_empty tolerates a missing filesystem. - pudl_deploy grows --deploy-gcs/--no-deploy-gcs and --deploy-s3/--no-deploy-s3 flags, threaded through resolve_build. - deploy-pudl.yml and build-pudl.yml expose matching workflow_dispatch inputs; build-pudl passes the resolved values to the batch job, and pudl_batch.sh forwards them to deploy-pudl and gates both trigger_deployment call sites. - Add unit tests and a release notes entry. Closes #5557

Co-authored-by: Kathryn Mazaitis <1158666+krivard@users.noreply.github.com>

zaneselvans · 2026-09-08T19:11:45Z

@krivard Updated some documentation to reflect the new deployment defaults.

krivard

I've clearly seen the word "only" too many times today and now it's lost all meaning

Co-authored-by: Kathryn Mazaitis <1158666+krivard@users.noreply.github.com>

github-project-automation Bot added this to Catalyst Megaproject Sep 5, 2026

github-project-automation Bot moved this to New in Catalyst Megaproject Sep 5, 2026

zaneselvans self-assigned this Sep 5, 2026

zaneselvans linked an issue Sep 5, 2026 that may be closed by this pull request

Disable S3 deployment for branch builds by default #5557

Closed

zaneselvans moved this from New to In progress in Catalyst Megaproject Sep 5, 2026

Base automatically changed from update-lockfiles to main September 5, 2026 21:57

zaneselvans force-pushed the deploy-optional branch from ffb37d2 to b3a0a02 Compare September 5, 2026 22:06

zaneselvans marked this pull request as draft September 5, 2026 22:30

zaneselvans changed the title ~~Skip S3 deployment on branch builds by default~~ WIP: Skip S3 deployment on branch builds by default Sep 5, 2026

zaneselvans force-pushed the deploy-optional branch 2 times, most recently from 46a51bb to bb9d4a9 Compare September 6, 2026 16:54

krivard self-requested a review September 8, 2026 17:08

krivard approved these changes Sep 8, 2026

View reviewed changes

zaneselvans moved this from In progress to In review in Catalyst Megaproject Sep 8, 2026

zaneselvans and others added 5 commits September 8, 2026 12:57

Apply batched suggestions from code review

555325e

Co-authored-by: Kathryn Mazaitis <1158666+krivard@users.noreply.github.com>

Clarify comment about boolean / null defaults & overrides

0a04419

Remove low-value test validating that Pydantic validates.

12fdf3a

Update docs to reflect new default deploy targets

6246648

zaneselvans force-pushed the deploy-optional branch from fafbcdc to 6246648 Compare September 8, 2026 19:10

zaneselvans marked this pull request as ready for review September 8, 2026 19:11

zaneselvans requested a review from krivard September 8, 2026 19:11

zaneselvans changed the title ~~WIP: Skip S3 deployment on branch builds by default~~ Skip S3 deployment on branch builds by default Sep 8, 2026

krivard approved these changes Sep 8, 2026

View reviewed changes

Comment thread docs/dev/nightly_data_builds.rst Outdated

Comment thread docs/dev/data_validation_reference.rst Outdated

Comment thread docs/dev/nightly_data_builds.rst Outdated

zaneselvans and others added 3 commits September 8, 2026 14:37

Update docs/dev/nightly_data_builds.rst

884c1a1

Co-authored-by: Kathryn Mazaitis <1158666+krivard@users.noreply.github.com>

Apply batched suggestions from code review

76274cf

Co-authored-by: Kathryn Mazaitis <1158666+krivard@users.noreply.github.com>

One more tiny wording tweak

af9f811

zaneselvans enabled auto-merge September 8, 2026 20:45

zaneselvans added this pull request to the merge queue Sep 8, 2026

Merged via the queue into main with commit 1180f7c Sep 8, 2026
12 checks passed

zaneselvans deleted the deploy-optional branch September 8, 2026 21:28

github-project-automation Bot moved this from In review to Done in Catalyst Megaproject Sep 8, 2026

		# Nightly and stable builds always deploy to both targets; branch builds
		# (workflow_dispatch) override these from the workflow inputs below.

		def test_deployment_plan_rejects_no_upload_target():
		"""A plan that uploads nowhere is rejected at construction time."""

Uh oh!

Uh oh!

Conversation

zaneselvans commented Sep 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Overview

What problem does this address?

What did you change?

Documentation

Testing

To-do list

Uh oh!

krivard left a comment

Choose a reason for hiding this comment

Uh oh!

krivard Sep 8, 2026

Choose a reason for hiding this comment

Uh oh!

krivard Sep 8, 2026

Choose a reason for hiding this comment

Uh oh!

krivard Sep 8, 2026

Choose a reason for hiding this comment

Uh oh!

zaneselvans Sep 8, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

krivard Sep 8, 2026

Choose a reason for hiding this comment

Uh oh!

zaneselvans Sep 8, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

krivard Sep 8, 2026

Choose a reason for hiding this comment

Uh oh!

zaneselvans Sep 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zaneselvans commented Sep 8, 2026

Uh oh!

zaneselvans commented Sep 8, 2026

Uh oh!

krivard left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

zaneselvans commented Sep 5, 2026 •

edited

Loading

zaneselvans Sep 8, 2026 •

edited

Loading