Builder: additional workflow steps #351

bkmartinjr · 2023-04-05T01:27:09Z

Implements the remaining build workflow steps as a fully automated process.

Pre-build host checks
Build SOMA
Validate SOMA
Create reports
Copy data to S3 bucket
Update the release.json to do the release
Clean up old releases
Copy logs to S3

This PR adds steps 5-8, although step 7 was present as a manually invoked sub-module prior to this PR.

Changes/additions:

Workflow sub-module to copy build to S3 (step 5). Fixes Builder workflow: add data copy step #294
Workflow sub-module to make release (step 6. Fixes Builder workflow: add release step #295
Workflow sub-module to copy logs to S3 (step 8). Fixes Builder workflow: add data copy step #294
Added main driver hooks to invoke these additional steps as part of the build

Note to reviewers: if you wish to test it, you can do so by creating a build dir, including a config.yaml that overrides the destination buckets. For example, the following config.yaml is a quick way to test with an abbreviated SOMA build:

cellxgene_census_S3_path: "s3://your-test-bucket/cell-census"
logs_S3_path: "s3://your-test-bucket/builder"
test_first_n: 15
host_validation_disable: True
release_cleanup_days: 10

Caveat: this approach requires that at least one release exist in the specified bucket, along with a release.json, etc. The release can be faked (i.e., the soma and h5ads paths can contain any empty file).

atolopko-czi

Looks good. Suggesting we upload reports too and two other very minor suggestions.

atolopko-czi · 2023-04-05T13:00:33Z

tools/cellxgene_census_builder/src/cellxgene_census_builder/data_copy.py

+        "--dryrun",
+        action=argparse.BooleanOptionalAction,
+        default=True,
+        help="Display, but do NOT perform actions. Useful for previewing actions. Default: True",


Suggested change

help="Display, but do NOT perform actions. Useful for previewing actions. Default: True",

help="Display, but do NOT perform S3 data update actions, including Census data upload, logs upload, and removal of older Census releases. Useful for previewing actions. Default: True",

This suggested edit does not make any sense in this test main. The only function provided by this CLI is to sync/copy a user-specified directory to S3 (any directory). It is completely agnostic about which directory and which S3 path.

Can you clarify what you are trying to accomplish?

Agreed, sorry, wrong place for the edit, this is not top-level entryp oint. If there's a better place to doc what dryrun does overall (config file), that's where I'd put it.

mmm...I'm not sure. It literally does what it says currently - short-circuits the data copy, at the very lowest level. It is primarily intended for testing purposes.

I'll give it some thought.

This is minor, don't give it too much thought. :). I simply figured it would be good to document in one place what dryrun does in total, since it affects a few of the pipeline's steps. "Skips all S3 updates" is sufficient.

Also, the config docs in build_state.py already contained:

"dryrun": False, # if True, will disable copy of data/logs/reports to S3 buckets

Expanded comment, and extended this config to apply to ALL S3 writes (including the write of release.json).

atolopko-czi · 2023-04-05T13:04:36Z

tools/cellxgene_census_builder/src/cellxgene_census_builder/__main__.py

@@ -58,6 +58,9 @@ def do_build(args: CensusBuildArgs, skip_completed_steps: bool = False) -> int:
        do_build_soma,
        do_validate_soma,
        do_create_reports,


We should also sync reports, and I don't think that's currently handled.

this was not in the build process the last time I checked, but is easily added.

It wasn't, I just realized the omission today while doing the release follow-up tasks. Ty!

atolopko-czi · 2023-04-05T13:08:28Z

tools/cellxgene_census_builder/src/cellxgene_census_builder/__main__.py

+            "uri": urlcat(args.config.cellxgene_census_S3_path, args.build_tag, "h5ads/"),
+            "s3_region": "us-west-2",
+        },
+    }


I suppose this release declaration could be moved into make_a_release, which would allow it to be unit tested.

Pushing all of the data layout assumptions down to the module feels like it pollutes the abstraction. I'll add some tests for the main.do_* methods instead.

added tests for all of the new workflow steps, called from the top-level

atolopko-czi

LGTM. Thank you adding the reports sync and additional tests.

atolopko-czi · 2023-04-05T18:51:16Z

tools/cellxgene_census_builder/src/cellxgene_census_builder/build_state.py

    "consolidate": True,
    "disable_dirty_git_check": True,
-    "dryrun": False,  # if True, will disable copy of data/logs/etc to S3 buckets
+    "dryrun": False,  # if True, will disable copy of data/logs/reports/release.json to S3 buckets. Will NOT disable local build, etc.


bkmartinjr added 4 commits April 4, 2023 18:23

additional workflow steps

9b753b1

pretty print the release manifest

04c61ee

wire up rest of workflow

60552c4

revise log handling

c14bc94

bkmartinjr marked this pull request as ready for review April 5, 2023 03:03

bkmartinjr requested review from ebezzi and atolopko-czi April 5, 2023 03:04

atolopko-czi requested changes Apr 5, 2023

View reviewed changes

bkmartinjr added 2 commits April 5, 2023 18:34

PR feedback

f6228b4

more comments

ea21d1b

bkmartinjr requested a review from atolopko-czi April 5, 2023 18:46

atolopko-czi approved these changes Apr 5, 2023

View reviewed changes

bkmartinjr merged commit c269115 into main Apr 5, 2023

bkmartinjr deleted the bkmartinjr/do-release-staging branch April 5, 2023 19:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Builder: additional workflow steps #351

Builder: additional workflow steps #351

bkmartinjr commented Apr 5, 2023 •

edited

Loading

atolopko-czi left a comment

atolopko-czi Apr 5, 2023

bkmartinjr Apr 5, 2023

atolopko-czi Apr 5, 2023

bkmartinjr Apr 5, 2023

atolopko-czi Apr 5, 2023

bkmartinjr Apr 5, 2023

bkmartinjr Apr 5, 2023

atolopko-czi Apr 5, 2023

bkmartinjr Apr 5, 2023

atolopko-czi Apr 5, 2023

bkmartinjr Apr 5, 2023

atolopko-czi Apr 5, 2023

bkmartinjr Apr 5, 2023

bkmartinjr Apr 5, 2023

atolopko-czi left a comment

atolopko-czi Apr 5, 2023

	help="Display, but do NOT perform actions. Useful for previewing actions. Default: True",
	help="Display, but do NOT perform S3 data update actions, including Census data upload, logs upload, and removal of older Census releases. Useful for previewing actions. Default: True",

Builder: additional workflow steps #351

Builder: additional workflow steps #351

Conversation

bkmartinjr commented Apr 5, 2023 • edited Loading

atolopko-czi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

atolopko-czi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bkmartinjr commented Apr 5, 2023 •

edited

Loading