Skip to content

Revamp duckdb-wasm extensions CI#10672

Merged
Mytherin merged 6 commits intoduckdb:mainfrom
carlopi:wasm_extensions_ci
Feb 15, 2024
Merged

Revamp duckdb-wasm extensions CI#10672
Mytherin merged 6 commits intoduckdb:mainfrom
carlopi:wasm_extensions_ci

Conversation

@carlopi
Copy link
Contributor

@carlopi carlopi commented Feb 14, 2024

CI on this PR is not actually checking anything, given the changed files are never invoked.

More proper test for this workflow is as follow:

  • set this branch as main for a fork (I have done it temporarily for https://github/carlopi/duckdb)
  • invoke the "DuckDB-Wasm Extensions" workflow (via WebUI or gh cli) on the fork, passing as parameters {'duckdb_ref' : 'v0.10.0', 's3_release' : false} (successful test run here: https://github.com/carlopi/duckdb/actions/runs/7912968453)
    here I would expect it should succeed, building the extensions, signing them with a fake key, setting up aws, and invoking aws s3 cp with --dryrun
    Sample of the log will be like (based on a past run):
autocomplete
(dryrun) upload: build/to_be_deployed/v0.10.0/wasm_eh/autocomplete.duckdb_extension.wasm.brotli to s3://duckdb-extensions/v0.10.0/wasm_eh/autocomplete.duckdb_extension.wasm
excel
(dryrun) upload: build/to_be_deployed/v0.10.0/wasm_eh/excel.duckdb_extension.wasm.brotli to s3://duckdb-extensions/v0.10.0/wasm_eh/excel.duckdb_extension.wasm
fts
(dryrun) upload: build/to_be_deployed/v0.10.0/wasm_eh/fts.duckdb_extension.wasm.brotli to s3://duckdb-extensions/v0.10.0/wasm_eh/fts.duckdb_extension.wasm
icu
(dryrun) upload: build/to_be_deployed/v0.10.0/wasm_eh/icu.duckdb_extension.wasm.brotli to s3://duckdb-extensions/v0.10.0/wasm_eh/icu.duckdb_extension.wasm
----
  • invoke the "DuckDB-Wasm Extensions" workflow (via WebUI or gh cli) on the fork, passing as parameters {'duckdb_ref' : 'v0.10.0', 's3_release' : true} (run is currently in flight here: https://github.com/carlopi/duckdb/actions/runs/7913085453)
    This should (if AWS variable are properly inherited from the environment) upload the extensions, otherwise fall back to --dryrun. Please double check you have AWS keys enabled in your repository if you want to try this.

s3_release set to false is not uploading nor signing anything, so it can be safely invoked also also on the duckdb/duckdb repo as an additional test AFTER merging this into main (before that this workflow is present in a older version performing an outdated logic).

My idea is that this PR can be reviewed, eventually merged, and then when it lands on main the workflow can be invoked first passing {'duckdb_ref' : 'v0.10.0', 's3_release' : false} checking the resulting paths make sense / stuff has not exploded, and then invoking it properly passing {'duckdb_ref' : 'v0.10.0', 's3_release' : true}.

Artifacts build during the first step are available after completion in the Summary page: https://github.com/carlopi/duckdb/actions/runs/7912968453.

@github-actions github-actions bot marked this pull request as draft February 14, 2024 20:18
@carlopi carlopi marked this pull request as ready for review February 14, 2024 20:18
@carlopi carlopi marked this pull request as draft February 14, 2024 20:30
@carlopi carlopi marked this pull request as ready for review February 14, 2024 22:15
@carlopi
Copy link
Contributor Author

carlopi commented Feb 14, 2024

Note that this CI job, invoked on request via workflow, is meant only to build the duckdb-wasm compatible extensions and store them on the official repository.

@github-actions github-actions bot marked this pull request as draft February 15, 2024 05:42
@carlopi carlopi marked this pull request as ready for review February 15, 2024 05:42
@github-actions github-actions bot marked this pull request as draft February 15, 2024 08:06
@carlopi carlopi marked this pull request as ready for review February 15, 2024 08:08
@github-actions github-actions bot marked this pull request as draft February 15, 2024 08:20
@carlopi carlopi marked this pull request as ready for review February 15, 2024 08:20
@carlopi carlopi requested a review from samansmink February 15, 2024 08:23
@carlopi
Copy link
Contributor Author

carlopi commented Feb 15, 2024

I think this makes sense, tried various times on my CI while polishing this, seems to work as expected.

Upload / signature will be properly tested once extensions are up by the duckdb-wasm CI on the branch bumping to duckdb v0.10.0.

@carlopi
Copy link
Contributor Author

carlopi commented Feb 15, 2024

Potentially an additional test, if we want to be extra sure, would be building duckdb-wasm extensions for a random commit (say commit the first landed PR after v0.10.0, that would be duckdb_ref = "8508978c55" + s3_release = true).

That should make that extensions ARE build, signed and deployed, but for an uninteresting commit hash (and we can then delete them from S3).

Copy link
Collaborator

@samansmink samansmink left a comment

Choose a reason for hiding this comment

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

Think this looks good, added 2 small comments

@carlopi
Copy link
Contributor Author

carlopi commented Feb 15, 2024

Thanks @samansmink, comments have been addressed.

@github-actions github-actions bot marked this pull request as draft February 15, 2024 09:43
@Mytherin Mytherin marked this pull request as ready for review February 15, 2024 09:45
@Mytherin Mytherin merged commit a0909db into duckdb:main Feb 15, 2024
krlmlr added a commit to duckdb/duckdb-r that referenced this pull request Mar 15, 2024
Merge pull request duckdb/duckdb#10658 from hannes/csvpathlength
Merge pull request duckdb/duckdb#10756 from Mytherin/preserveinsertionordermemory
Merge pull request duckdb/duckdb#10746 from samansmink/enable-azure-autoload
Merge pull request duckdb/duckdb#10747 from maiadegraaf/list_reverse_bug
Merge pull request duckdb/duckdb#10748 from taniabogatsch/capi-tests
Merge pull request duckdb/duckdb#10739 from peterboncz/pb/immmedate_mode_only_in_non_autocommit
Merge pull request duckdb/duckdb#10688 from Tmonster/union_exclude
Merge pull request duckdb/duckdb#10710 from samansmink/comment-on-column
Merge pull request duckdb/duckdb#10725 from hawkfish/fuzzer-preceding-frame
Merge pull request duckdb/duckdb#10723 from hawkfish/fuzzer-null-timestamp
Merge pull request duckdb/duckdb#10436 from taniabogatsch/map-fixes
Merge pull request duckdb/duckdb#10587 from kryonix/main
Merge pull request duckdb/duckdb#10738 from TinyTinni/fix-assert-in-iscntrl
Merge pull request duckdb/duckdb#10708 from carlopi/ci_fixes
Merge pull request duckdb/duckdb#10726 from hawkfish/fuzzer-to-weeks
Merge pull request duckdb/duckdb#10727 from hawkfish/fuzzer-window-bind
Merge pull request duckdb/duckdb#10733 from TinyTinni/remove-static-string
Merge pull request duckdb/duckdb#10715 from Tishj/python_tpch_regression_rework
Merge pull request duckdb/duckdb#10728 from hawkfish/fuzzer-argminmax-decimal
Merge pull request duckdb/duckdb#10717 from carlopi/fix_extension_deploy
Merge pull request duckdb/duckdb#10694 from Mytherin/castquerylocation
Merge pull request duckdb/duckdb#10448 from peteraisher/feature/use-assertThrows-for-jdbc-tests
Merge pull request duckdb/duckdb#10691 from Mytherin/issue10685
Merge pull request duckdb/duckdb#10684 from Mytherin/distincton
Merge pull request duckdb/duckdb#9539 from Tishj/timestamp_unit_to_tz
Merge pull request duckdb/duckdb#10341 from Tmonster/tpch_ingestion_benchmark
Merge pull request duckdb/duckdb#10689 from Mytherin/juliaversion
Merge pull request duckdb/duckdb#10669 from Mytherin/skippedtests
Merge pull request duckdb/duckdb#10679 from Tishj/reenable_window_rows_overflow
Merge pull request duckdb/duckdb#10672 from carlopi/wasm_extensions_ci
Merge pull request duckdb/duckdb#10660 from szarnyasg/update-storage-info-for-v0100
Merge pull request duckdb/duckdb#10643 from bleskes/duck_transaction_o11y
Merge pull request duckdb/duckdb#10654 from carlopi/fix_10548
Merge pull request duckdb/duckdb#10650 from hannes/noprintf
Merge pull request duckdb/duckdb#10649 from Mytherin/explicitenumnumbers
@carlopi carlopi deleted the wasm_extensions_ci branch May 7, 2024 08:09
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.

3 participants