packaging: Integrate github workflows to automate the air-gapping process of wave university

h2oai / wave

Realtime Web Apps and Dashboards for Python and R

https://wave.h2o.ai

Apache License 2.0

3.9k stars 323 forks source link

packaging: Integrate github workflows to automate the air-gapping process of wave university #2181

Closed sulhicader closed 4 months ago

sulhicader commented 8 months ago

The PR fulfills these requirements: (check all the apply)

[ ] It's submitted to the main branch.
[ ] When resolving a specific issue, it's referenced in the PR's title (e.g. feat: Add a button #xxx, where "xxx" is the issue number).
[ ] When resolving a specific issue, the PR description includes Closes #xxx, where "xxx" is the issue number.
[ ] If changes were made to ui folder, unit tests (make test) still pass.
[ ] New/updated tests are included

This closes issue #2163

mturoci commented 7 months ago

Thanks for the PR @sulhicader!

What's the reason for splitting into multiple files/workflows? It seems like an unnecessary abstraction that introduces plenty of duplication which will be hard to maintain for a few relatively trivial tasks - build and publish docker/helm.

Can we put it into existing workflow files instead? Also, it seems like you are only doing this for Wave University app so there is no need to make it overly parametrized, and hardcoding for simplicity should be fine for now.

sulhicader commented 7 months ago

@mturoci Thanks you for the inputs.

.github/workflows/wave-bundle-docker-build-publish.yaml, .github/workflows/wave-bundle-helm-release.yaml workflows will going to be common for all the apps going to pusblish from wave repo ( for now university and tour ). Also I mainly using the workflows from https://github.com/h2oai/wave-template/tree/main/.github/workflows template repo and in future if any additions comes to it, it will be easier to reflect those here. Here new workflows are triggered from .github/workflows/publish-university.yml which is existing workflow. As you suggest We can reduce the workflow files but putting into one file, but it also deviate from the standard template. WDYT?

mturoci commented 7 months ago

The template is designed to build an arbitrary app for scratch, not to be integrated into existing workflows like here so it doesn't make sense to introduce it since all the apps have their builds already defined.

The builds only need to be adjusted to use h2o cli instead of plain zip.
Add steps to push to ECR / Helm as needed.

Correct me if I missed something in ^^.

Also, if anything changes, I can guarantee you (or anyone) will be able to resolve it much faster in 2 files with proper context than juggling across 6 files.

sulhicader commented 7 months ago

Sure @mturoci, I am working on this.

sulhicader commented 5 months ago

Hi @mturoci, I addressed the review comments and took the functionalities into one script. Please feel free to add comments if any changes needed.