Workflow to Push Containers to Canary #68
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Basic service scaffolding using Scala Play framework
# Conflicts: # build.sbt # hub/app/controllers/FrontendController.scala # hub/app/views/index.scala.html # hub/public/images/favicon.png
Adds a readme, and keeps the docker container active so the parquet table can be accessed.
…nto docker-init
Create svelte project and build it using play for deployment
## Summary Scope all the Github workflow trigger paths regarding workflows to self to not unnecessarily trigger all the other workflows.  See Slack [thread](https://zipline-2kh4520.slack.com/archives/C072LUA50KA/p1734555635936189) ## Checklist - [ ] Added Unit Tests - [ ] Covered by existing CI - [ ] Integration tested - [ ] Documentation update <!-- av pr metadata This information is embedded by the av CLI when creating PRs to track the status of stacks when using Aviator. Please do not delete or edit this section of the PR. ``` {"parent":"main","parentHead":"","trunk":"main"} ``` --> Co-authored-by: Sean Lynch <[email protected]>
## Summary [`pnpm`](https://pnpm.io/) vs `npm` - Pros - Integrated (interactive) version manager - `pnpm up-deps` / (aliased `pnpm update -r -i --latest`) - `npm outdated` / `npm update --save-dev --save` only adhere to `package.json` semver - Must manually modify versions for major versions (ex. Vite 5 => 6) - Can use [`npx npm-check-updates`](https://github.com/raineorshine/npm-check-updates) package to provide similar functionality - Efficient Disk space - Workspace support - npm added [support](https://docs.npmjs.com/cli/v8/using-npm/workspaces) but not aware of any major projects using it - Typically [faster installs](https://pnpm.io/benchmarks) than npm - Slightly better developer ergonomics (`pnpm dev` vs `npm run dev`) - [Used](https://pnpm.io/users) by many large projects/companies (Vite, Svelte, etc) - Cons - Additional install (`npm` included with Node.js) - Synk [support](https://docs.snyk.io/supported-languages-package-managers-and-frameworks/javascript/javascript-for-open-source?_gl=1*1gi9low*_gcl_au*MTA2ODIxOTcxNy4xNzM0Mzg4MTY2*_ga*MTEzMzg2NzM3NS4xNzM0MDIzMDg5*_ga_X9SH3KP7B4*MTczNDU0MzI4Mi40LjAuMTczNDU0MzI4Mi4wLjAuMA..#pnpm) is currently in [preview](https://app.snyk.io/org/varant-zlai/manage/beta-features) - package version overrides not currently applying with Synk (configuration, preview status of integration, ...). Likely because snyk is configured to scan `package-lock.json` (npm) and not `pnpm-lock.yaml` (pnpm). ([here](https://app.snyk.io/org/varant-zlai/project/f4bdc116-d05b-4937-96b5-b1f9a02872e5)) ---   ## Checklist - [ ] Added Unit Tests - [x] Covered by existing CI - [ ] Integration tested - [ ] Documentation update <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Documentation** - Updated README files to reflect the use of `pnpm` as the recommended package manager, including installation instructions and best practices. - **New Features** - Transitioned scripts in the project to utilize `pnpm` for various tasks including development, building, and testing. - **Configuration** - Modified Playwright configuration and other build configurations to use `pnpm` for starting the web server and managing dependencies. - **Dependency Updates** - Updated Spark dependency version from `3.5.0` to `3.5.1` and Jackson version from `2.15.1` to `2.15.2`. <!-- end of auto-generated comment: release notes by coderabbit.ai --> <!-- av pr metadata This information is embedded by the av CLI when creating PRs to track the status of stacks when using Aviator. Please do not delete or edit this section of the PR. ``` {"parent":"main","parentHead":"","trunk":"main"} ``` --> --------- Co-authored-by: Sean Lynch <[email protected]> Co-authored-by: ken-zlai <[email protected]>
## Summary
Setup CI workflow to run `check` (svelte-check / type errors), `lint`
(prettier format and eslint), and successfully build the project. We'll
enable `test:unit` (vitest) and `test:integration` (playwright) once
there is more API separation / setup.
## Checklist
- [ ] Added Unit Tests
- [ ] Covered by existing CI
- [ ] Integration tested
- [ ] Documentation update
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
- **New Features**
- Introduced a new GitHub Actions workflow for automated frontend
testing.
- **Chores**
- Configured the workflow to trigger on pushes and pull requests to the
main branch.
- Added steps for dependency installation and project build in the
workflow.
- Included setup for Playwright browsers and Svelte checks, with future
enhancements planned.
- Updated `.prettierignore` to exclude specific files and directories
from formatting.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
<!-- av pr metadata
This information is embedded by the av CLI when creating PRs to track
the status of stacks when using Aviator. Please do not delete or edit
this section of the PR.
```
{"parent":"main","parentHead":"","trunk":"main"}
```
-->
---------
Co-authored-by: Sean Lynch <[email protected]>
#134) ## Summary Right now our search endpoint only returns joins. I change the code to account for this by showing all results as a join. I have also only made `risk.user_transactions.txn_join` clickable, since it is the only one to have data. In the future, we should add an `entityType` to search results to categorize them on the frontend. ## Checklist - [ ] Added Unit Tests - [ ] Covered by existing CI - [ ] Integration tested - [ ] Documentation update <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Enhanced rendering of search results in the navigation bar with improved handling of entity properties. - **Bug Fixes** - Fixed conditional display logic for command items based on entity names. - **Refactor** - Streamlined the selection logic and rendering process for search results by replacing the `model` variable with `entity`. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary Reverting back to `npm` due to Snyk UI not supporting `pnpm.overrides` in `package.json` (best source appears to be this [comment](snyk/nodejs-lockfile-parser#111 (comment))). Sounds like the CLI might support it at this time, which means the Snyk [github action](https://github.com/snyk/actions/tree/master/node) might work, but not worth exploring further at this time. Also not sure if Vanta requires Snyk UI for compliance. ## Checklist - [ ] Added Unit Tests - [ ] Covered by existing CI - [ ] Integration tested - [ ] Documentation update <!-- av pr metadata This information is embedded by the av CLI when creating PRs to track the status of stacks when using Aviator. Please do not delete or edit this section of the PR. ``` {"parent":"main","parentHead":"","trunk":"main"} ``` --> Co-authored-by: Sean Lynch <[email protected]>
…x. package.json or other config changes) (#151) ## Summary Run `test_frontend` workflow for `package.json` (and other config files) changes ## Checklist - [ ] Added Unit Tests - [ ] Covered by existing CI - [ ] Integration tested - [ ] Documentation update <!-- av pr metadata This information is embedded by the av CLI when creating PRs to track the status of stacks when using Aviator. Please do not delete or edit this section of the PR. ``` {"parent":"main","parentHead":"","trunk":"main"} ``` --> Co-authored-by: Sean Lynch <[email protected]>
## Summary - Step 0 of https://app.asana.com/0/1208949807589885/1208951092959581/f - The end goal of this PR stack will be to support spark reading from and writing to BigQuery. To do this we'll leverage the `Format` abstraction to delegate out certain catalog operations such as listing partitions. I will do a followup of this to introduce `BigQueryFormatProvider` that will cover the bigquery cases. - This PR is simply just factoring out `Format`, and also modifying its class signature and cleaning up the reflection classloading. ## Checklist - [ ] Added Unit Tests - [ ] Covered by existing CI - [ ] Integration tested - [ ] Documentation update <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Introduced a trait-based architecture for managing various table formats in Apache Spark. - Added support for dynamic loading of format providers at runtime. - **Bug Fixes** - Enhanced partition retrieval logic for Iceberg and DeltaLake formats to address known issues. - **Refactor** - Streamlined the `TableUtils` class and improved error handling for unsupported operations. - Removed redundant code and commented sections for better maintainability. - **Tests** - Added a new test case for dynamic class loading of the table format provider. <!-- end of auto-generated comment: release notes by coderabbit.ai --> <!-- av pr metadata This information is embedded by the av CLI when creating PRs to track the status of stacks when using Aviator. Please do not delete or edit this section of the PR. ``` {"parent":"main","parentHead":"","trunk":"main"} ``` -->
## Summary - New class-based `Api` interface - Support passing custom `fetch` to allow relative URLs from SvelteKit [load()](https://svelte.dev/docs/kit/web-standards#Fetch-APIs) functions - Defaults to global `fetch` ([window.fetch](https://developer.mozilla.org/en-US/docs/Web/API/Window/fetch) on browser, global [fetch](https://nodejs.org/dist/v18.20.5/docs/api/globals.html#fetch) on Node.js) - Improve response parsing - Better error handling - TODO: Detect date strings and create `Date` instances (using `parse` from [@layerstack/utils](https://www.layerstack.dev/docs/utils/json)) - Enable setting other api context in the future, such as `Authorization` header for access tokens - Proxy all calls through frontend server - Examples: - `http://localhost:5173/api/*` => `http://localhost:9000/api/v1/*` - `http://app.zipline.ai/api/*` => `http://app.zipline.ai:9000/api/v1/*` - Resolves - CORS - Consistent URL handling whether issuing requests from browser (ex. `+page.svelte`) or frontend server (ex `+page.server.ts`) ## Test results  ## Checklist - [ ] Added Unit Tests - [x] Covered by existing CI - [ ] Integration tested - [ ] Documentation update <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Summary by CodeRabbit - **New Features** - Enhanced installation instructions in the README, including the creation of a `.env` file for configuration. - Introduced a proxy for API requests to handle CORS issues. - **Improvements** - Refactored API handling to use a new `Api` class, improving organization and encapsulation of API interactions. - Updated search functionality in the NavigationBar to utilize the new API class. - Enhanced error handling during data fetching across various components. - **Documentation** - Clarified setup details in the README file. <!-- end of auto-generated comment: release notes by coderabbit.ai --> <!-- av pr metadata This information is embedded by the av CLI when creating PRs to track the status of stacks when using Aviator. Please do not delete or edit this section of the PR. ``` {"parent":"main","parentHead":"","trunk":"main"} ``` --> --------- Co-authored-by: Sean Lynch <[email protected]>
…ons` (#133) ## Summary Replace `svelte-hero-icons` and `@zipline-ai/icons` with `unplugin-icons` which improve: - Simplifies using other icon sets (ex. Lucide) - Simplifies adding custom icons (see: [vite.confg.ts](https://github.com/zipline-ai/chronon/pull/133/files#diff-97c6e2c429ec483132c584137a18a1ade2ff6c3f4f6485f4a83db604d0323d7cR11-R17) - Is well maintained - [unplugin-icons](https://www.npmjs.com/package/svelte-hero-icons) last released 2 days ago - [svelte-hero-icons](https://www.npmjs.com/package/svelte-hero-icons) last released 5 months ago) - Used by many frameworks using Vite including Svelte, Vue, and React. - Installing the `vscode-iconify` [plugin](https://github.com/antfu/vscode-iconify) makes the experience 1000% better. Also browsing iconsets using [icones.js.org](https://icones.js.org/collection/heroicons) which have quick copy/paste for "Unplugin Icons". Before | After --- | ---  |  ## Checklist - [ ] Added Unit Tests - [ ] Covered by existing CI - [ ] Integration tested - [ ] Documentation update <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Introduced new icon packages for enhanced visual elements in the application. - **Improvements** - Updated multiple components to utilize new icon imports, streamlining icon rendering and improving maintainability. - **Bug Fixes** - Removed outdated icon dependencies to prevent potential conflicts and ensure compatibility with new imports. - **Documentation** - Added type exports from the new icon plugin for better type accessibility across the application. <!-- end of auto-generated comment: release notes by coderabbit.ai --> <!-- av pr metadata This information is embedded by the av CLI when creating PRs to track the status of stacks when using Aviator. Please do not delete or edit this section of the PR. ``` {"parent":"main","parentHead":"","trunk":"main"} ``` --> --------- Co-authored-by: Sean Lynch <[email protected]>
…tc) (#150) ## Summary Due to some of our odd issues (out of memory, etc) I thought it would be helpful to be on the latest Vite and Svelte versions. I also added `npm run up-deps` similar to pnpm PR but leveraging [npm-check-updates](https://github.com/raineorshine/npm-check-updates) package to make bumping versions easier (and interactive). This updates all dependencies except `bits-ui` (0.21.16 => 0.22.0) and `svelte-radix` (1.1.1 => 2.0.1) which are used by shadcn-svelte. They are probably safe to update (especially bits-ui) but would require more visual validation. I had to add a `vite` override as some internal deps of `vitest` were still requesting `vite@5` (and oddly this isn't an issue when using `pnpm` as I have a similar vite 6 setup on other repos. ## Checklist - [ ] Added Unit Tests - [ ] Covered by existing CI - [ ] Integration tested - [ ] Documentation update <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added a new script for dependency management. - **Updates** - Multiple package versions updated to enhance performance and compatibility, including key libraries such as Svelte, Playwright, and TypeScript. <!-- end of auto-generated comment: release notes by coderabbit.ai --> <!-- av pr metadata This information is embedded by the av CLI when creating PRs to track the status of stacks when using Aviator. Please do not delete or edit this section of the PR. ``` {"parent":"main","parentHead":"","trunk":"main"} ``` --> --------- Co-authored-by: Sean Lynch <[email protected]>
## Summary
Noticed that the search was behaving weird and there were sometimes
duplicates in the list when typing. This is because the `{#each}` block
was not keyed.
## Checklist
- [ ] Added Unit Tests
- [ ] Covered by existing CI
- [ ] Integration tested
- [ ] Documentation update
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
- **New Features**
- Enhanced rendering performance of search results in the navigation
bar.
- Conditional disabling of command items based on entity names for
improved interactivity.
- Improved visibility control for the command dialog.
- **Bug Fixes**
- Maintained consistent error handling and event management for keyboard
shortcuts.
- **Documentation**
- Updated type definitions for exported properties in the navigation bar
component.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary - This is part 1 of the saga to supportBigQuery reads. https://app.asana.com/0/1208949807589885/1208951092959581/f - There are sibling PR's that address BigQuery cataloging: - - #145 - - #146 - - #147 - - #148 In terms of functionality, they are not technically dependent on one another for code completeness, however will need to work in concert to fully support BQ as a data source. - This PR is the first step to supporting BigQuery partition pushdown. Partition filters are handled separate from predicates, see: https://github.com/GoogleCloudDataproc/spark-bigquery-connector?tab=readme-ov-file#partitioned-tables - We need to follow up this PR by setting the partition filter in the read option. Since this is a significant change, we'll break it up into steps so we can test incrementally. ## Checklist - [ ] Added Unit Tests - [x] Covered by existing CI - [ ] Integration tested - [ ] Documentation update <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Enhanced DataFrame querying capabilities with additional filtering options. - Improved exception handling and logging for backfill operations. - **Bug Fixes** - Refined logic for data filtering and retrieval in join operations. - **Documentation** - Updated method signatures to reflect new parameters and functionality. <!-- end of auto-generated comment: release notes by coderabbit.ai --> <!-- av pr metadata This information is embedded by the av CLI when creating PRs to track the status of stacks when using Aviator. Please do not delete or edit this section of the PR. ``` {"parent":"main","parentHead":"","trunk":"main"} ``` -->
## Summary
- Following the `Format` abstraction, we'll introduce something new for
BigQuery that'll provide a combination of read/write support as well as
catalog operations for partitioning.
- The code for listing partitions uses the spark-bigquery-connector and
runs SQL against the information schema.
## Checklist
- [ ] Added Unit Tests
- [ ] Covered by existing CI
- [ ] Integration tested
- [ ] Documentation update
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
## Summary by CodeRabbit
- **New Features**
- Introduced functionality for reading and writing data formats specific
to Google Cloud Platform's BigQuery.
- Added support for determining table types and retrieving partition
information for BigQuery tables.
- **Bug Fixes**
- Enhanced dependency management for the `cloud_gcp` project to improve
build consistency.
- **Documentation**
- Updated dependency declarations for clarity and better project
structure.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
<!-- av pr metadata
This information is embedded by the av CLI when creating PRs to track
the status of stacks when using Aviator. Please do not delete or edit
this section of the PR.
```
{"parent":"main","parentHead":"","trunk":"main"}
```
-->
## Summary
## Checklist
- [ ] Added Unit Tests
- [ ] Covered by existing CI
- [ ] Integration tested
- [ ] Documentation update
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
- **New Features**
- Added new library dependencies to enhance Google Cloud Storage
integration.
- Introduced a new `GCS` class for handling partitioning in Google Cloud
Storage.
- **Improvements**
- Updated `BigQueryFormat` to refine table reading and processing logic.
- Enhanced error handling for external table definitions and improved
control flow for table metadata access.
- **Bug Fixes**
- Improved error handling for missing database specifications in
partition methods.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
<!-- av pr metadata
This information is embedded by the av CLI when creating PRs to track
the status of stacks when using Aviator. Please do not delete or edit
this section of the PR.
```
{"parent":"main","parentHead":"","trunk":"main"}
```
-->
## Summary Regarding this security issue: https://github.com/zipline-ai/chronon/security/dependabot/8, we are told that [nanoid](https://www.npmjs.com/package/nanoid) Version 3.3.8 and 5.0.9 are fixed. Initially I tried using 5.0.9, but ran into CI issues in our github actions check when running `npm run check`. Upon research, something in 5.0.9 introduced changes to trigger this. I've opted to use 3.3.8 since it fixes the security issue. ## Checklist - [ ] Added Unit Tests - [ ] Covered by existing CI - [ ] Integration tested - [ ] Documentation update <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Introduced a new dependency for improved functionality. - **Chores** - Updated frontend testing workflow to enhance dependency installation process. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary Our Spark on Flink code doesn't include registering UDFs which is a gap compared to our Spark structured streaming implementation. This PR adds support for this. I've skipped registering of UDFs in derivations - can add this either as part of this PR or in a follow up when the need arises. To confirm the jar registration works I added a directory in quickstart with a couple of example UDFs and register that jar in my SparkExprEvalFnTest to confirm things work as expected. ## Checklist - [X] Added Unit Tests - [ ] Covered by existing CI - [ ] Integration tested - [ ] Documentation update <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Enhanced functionality for evaluating Spark SQL expressions with additional setup configurations. - Introduced new user-defined functions (UDFs) for testing purposes. - Added support for Hive UDF registration in the `CatalystUtil` class. - **Bug Fixes** - Improved error handling for setup statements in `CatalystUtil`. - **Tests** - Added new test methods to validate UDF functionality and integration with the `CatalystUtil` framework. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (1)
.github/workflows/push_to_canary.yaml(3 hunks)
🧰 Additional context used
🪛 yamllint (1.35.1)
.github/workflows/push_to_canary.yaml
[error] 50-50: trailing spaces
(trailing-spaces)
🔇 Additional comments (1)
.github/workflows/push_to_canary.yaml (1)
13-18: 🛠️ Refactor suggestion
Move repository paths to secrets
Hardcoded repository paths reduce flexibility across environments.
- AWS_QUICKSTART_REPOSITORY: zipline-ai/quickstart
- AWS_APP_REPOSITORY: zipline-ai/app
- AWS_FRONTEND_REPOSITORY: zipline-ai/frontend
- GAR_QUICKSTART_REPOSITORY: canary-images/quickstart
- GAR_APP_REPOSITORY: canary-images/app
- GAR_FRONTEND_REPOSITORY: canary-images/frontend
+ AWS_QUICKSTART_REPOSITORY: ${{ secrets.AWS_QUICKSTART_REPOSITORY }}
+ AWS_APP_REPOSITORY: ${{ secrets.AWS_APP_REPOSITORY }}
+ AWS_FRONTEND_REPOSITORY: ${{ secrets.AWS_FRONTEND_REPOSITORY }}
+ GAR_QUICKSTART_REPOSITORY: ${{ secrets.GAR_QUICKSTART_REPOSITORY }}
+ GAR_APP_REPOSITORY: ${{ secrets.GAR_APP_REPOSITORY }}
+ GAR_FRONTEND_REPOSITORY: ${{ secrets.GAR_FRONTEND_REPOSITORY }}Likely invalid or redundant comment.
| id: build-aws-app | ||
| shell: bash | ||
| run: | ||
| docker build "." -f "docker-init/Dockerfile" -t "aws-app-image:latest" --build-arg "AWS_DEFAULT_REGION=${{secrets.AWS_REGION}}" --build-arg "DYNAMO_ENDPOINT=https://dynamodb.${{secrets.AWS_REGION}}.amazonaws.com" --build-arg "PLAY_HTTP_SECRET_KEY=my_fake_chronon_monitoring_hub_http_secret_key" --build-arg "JAVA_OPTS=-Xms16g -Xmx16g" --build-arg "SPARK_JAR=/app/cli/spark.jar" --build-arg "CLOUD_AWS_JAR=/app/cli/cloud_aws.jar" --build-arg "ONLINE_CLASS=ai.chronon.integrations.aws.AwsApiImpl" |
There was a problem hiding this comment.
Move sensitive build arguments to secrets
Hardcoded secrets in build args pose a security risk.
- --build-arg "PLAY_HTTP_SECRET_KEY=my_fake_chronon_monitoring_hub_http_secret_key" --build-arg "JAVA_OPTS=-Xms16g -Xmx16g"
+ --build-arg "PLAY_HTTP_SECRET_KEY=${{ secrets.PLAY_HTTP_SECRET_KEY }}" --build-arg "JAVA_OPTS=${{ secrets.JAVA_OPTS }}"Also applies to: 114-114, 121-121
Comment on lines
103
to
122
|
|
||
| - name: Build AWS App Image | ||
| id: build-aws-app | ||
| shell: bash | ||
| run: | ||
| docker build "." -f "docker-init/Dockerfile" -t "aws-app-image:latest" --build-arg "AWS_DEFAULT_REGION=${{secrets.AWS_REGION}}" --build-arg "DYNAMO_ENDPOINT=https://dynamodb.${{secrets.AWS_REGION}}.amazonaws.com" --build-arg "PLAY_HTTP_SECRET_KEY=my_fake_chronon_monitoring_hub_http_secret_key" --build-arg "JAVA_OPTS=-Xms16g -Xmx16g" --build-arg "SPARK_JAR=/app/cli/spark.jar" --build-arg "CLOUD_AWS_JAR=/app/cli/cloud_aws.jar" --build-arg "ONLINE_CLASS=ai.chronon.integrations.aws.AwsApiImpl" | ||
|
|
||
| - name: Build GCP App Image | ||
| id: build-gcp-app | ||
| shell: bash | ||
| run: | ||
| docker build "." -f "docker-init/Dockerfile" -t "gcp-app-image:latest" --build-arg "GCP_DEFAULT_REGION=${{secrets.GCP_REGION}}" --build-arg "BIGTABLE_ENDPOINT=https://${{secrets.GCP_REGION}}-bigtable.googleapis.com" --build-arg "PLAY_HTTP_SECRET_KEY=my_fake_chronon_monitoring_hub_http_secret_key" --build-arg "JAVA_OPTS=-Xms16g -Xmx16g" --build-arg "SPARK_JAR=/app/cli/spark.jar" --build-arg "CLOUD_GCP_JAR=/app/cli/cloud_gcp.jar" --build-arg "ONLINE_CLASS=ai.chronon.integrations.cloud_gcp.GcpApiImpl" | ||
|
|
||
|
|
||
| - name: Build Frontend Image | ||
| id: build-frontend | ||
| shell: bash | ||
| run: | ||
| docker build "." -f "docker-init/frontend/Dockerfile" -t "frontend-image:latest" --build-arg "API_BASE_URL=http://app:9000" | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add container vulnerability scanning
Add security scanning before pushing images.
- name: Scan container images
uses: aquasecurity/trivy-action@master
with:
image-ref: |
aws-app-image:latest
gcp-app-image:latest
frontend-image:latest
format: 'table'
exit-code: '1'
ignore-unfixed: true
vuln-type: 'os,library'
severity: 'CRITICAL,HIGH'
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
.github/workflows/push_to_canary.yaml (1)
152-152: Improve image versioning strategyStatic 'main' tag makes rollbacks difficult.
- IMAGE_TAG: main + IMAGE_TAG: canary-${{ github.run_number }}Also applies to: 172-172, 205-205, 223-223, 241-241
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (1)
.github/workflows/push_to_canary.yaml(4 hunks)
🧰 Additional context used
🪛 yamllint (1.35.1)
.github/workflows/push_to_canary.yaml
[error] 50-50: trailing spaces
(trailing-spaces)
🔇 Additional comments (2)
.github/workflows/push_to_canary.yaml (2)
121-121: 🛠️ Refactor suggestion
Make API_BASE_URL configurable
API URL should be configurable per environment.
-docker build "." -f "docker-init/frontend/Dockerfile" -t "frontend-image:latest" --build-arg "API_BASE_URL=http://app:9000"
+docker build "." -f "docker-init/frontend/Dockerfile" -t "frontend-image:latest" --build-arg "API_BASE_URL=${{secrets.API_BASE_URL}}"Likely invalid or redundant comment.
189-191: 🛠️ Refactor suggestion
Move GCP service account configuration to secrets
Service account details should be fully secured.
- project_id: ${{secrets.GCP_PROJECT_ID}}
- workload_identity_provider: projects/${{secrets.GCP_PROJECT_NUMBER}}/locations/global/workloadIdentityPools/github-actions/providers/github
- service_account: githubactions@${{secrets.GCP_PROJECT_ID}}.iam.gserviceaccount.com
+ project_id: ${{secrets.GCP_PROJECT_ID}}
+ workload_identity_provider: ${{secrets.GCP_WORKLOAD_IDENTITY_PROVIDER}}
+ service_account: ${{secrets.GCP_SERVICE_ACCOUNT}}Likely invalid or redundant comment.
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
.github/workflows/push_to_canary.yaml (2)
153-154: Improve image versioning strategyUsing static 'canary' tag makes rollbacks difficult.
- IMAGE_TAG: canary + IMAGE_TAG: canary-${{ github.run_number }}Also applies to: 165-165, 185-185, 218-218, 236-236, 254-254
55-57: Add error handling to SBT build stepsAdd proper error handling for each SBT command.
set -eo pipefail -sbt clean && sbt assembly -sbt "project frontend" buildFrontend +sbt clean && sbt assembly || { + echo "::error::SBT assembly failed" + exit 1 +} +sbt "project frontend" buildFrontend || { + echo "::error::Frontend build failed" + exit 1 +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (1)
.github/workflows/push_to_canary.yaml(3 hunks)
🧰 Additional context used
🪛 yamllint (1.35.1)
.github/workflows/push_to_canary.yaml
[error] 50-50: trailing spaces
(trailing-spaces)
🔇 Additional comments (1)
.github/workflows/push_to_canary.yaml (1)
124-135: LGTM! Good security practice
Adding container vulnerability scanning is a great security measure.
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
.github/workflows/push_to_canary.yaml (1)
139-139: Improve image versioning strategyStatic 'canary' tag makes rollbacks difficult.
- IMAGE_TAG: canary + IMAGE_TAG: canary-${{ github.run_number }}Also applies to: 151-151, 171-171
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (1)
.github/workflows/push_to_canary.yaml(3 hunks)
🧰 Additional context used
🪛 yamllint (1.35.1)
.github/workflows/push_to_canary.yaml
[error] 50-50: trailing spaces
(trailing-spaces)
🔇 Additional comments (2)
.github/workflows/push_to_canary.yaml (2)
188-190: 🛠️ Refactor suggestion
Move GCP service account configuration to secrets
Service account details should be fully secured.
- project_id: ${{secrets.GCP_PROJECT_ID}}
- workload_identity_provider: projects/${{secrets.GCP_PROJECT_NUMBER}}/locations/global/workloadIdentityPools/github-actions/providers/github
- service_account: githubactions@${{secrets.GCP_PROJECT_ID}}.iam.gserviceaccount.com
+ project_id: ${{secrets.GCP_PROJECT_ID}}
+ workload_identity_provider: ${{secrets.GCP_WORKLOAD_IDENTITY_PROVIDER}}
+ service_account: ${{secrets.GCP_SERVICE_ACCOUNT}}Likely invalid or redundant comment.
13-18: 🛠️ Refactor suggestion
Move repository paths to GitHub secrets
Hardcoded repository paths reduce flexibility and maintainability.
- AWS_QUICKSTART_REPOSITORY: zipline-ai/quickstart
- AWS_APP_REPOSITORY: zipline-ai/app
- AWS_FRONTEND_REPOSITORY: zipline-ai/frontend
- GAR_QUICKSTART_REPOSITORY: canary-images/quickstart
- GAR_APP_REPOSITORY: canary-images/app
- GAR_FRONTEND_REPOSITORY: canary-images/frontend
+ AWS_QUICKSTART_REPOSITORY: ${{ secrets.AWS_QUICKSTART_REPOSITORY }}
+ AWS_APP_REPOSITORY: ${{ secrets.AWS_APP_REPOSITORY }}
+ AWS_FRONTEND_REPOSITORY: ${{ secrets.AWS_FRONTEND_REPOSITORY }}
+ GAR_QUICKSTART_REPOSITORY: ${{ secrets.GAR_QUICKSTART_REPOSITORY }}
+ GAR_APP_REPOSITORY: ${{ secrets.GAR_APP_REPOSITORY }}
+ GAR_FRONTEND_REPOSITORY: ${{ secrets.GAR_FRONTEND_REPOSITORY }}Likely invalid or redundant comment.
| exit 1 | ||
| } | ||
| docker push "${{env.GAR_APP_REGISTRY}}:${{github.sha}}" |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add container vulnerability scanning
Add security scanning before pushing to registries.
- name: Scan container images
uses: aquasecurity/trivy-action@master
with:
image-ref: |
aws-app-image:latest
gcp-app-image:latest
frontend-image:latest
format: 'table'
exit-code: '1'
ignore-unfixed: true
vuln-type: 'os,library'
severity: 'CRITICAL,HIGH'
Comment on lines
5
to
6
| # branches: | ||
| # - 'main' |
There was a problem hiding this comment.
Enable workflow trigger for main branch
Uncomment the branch trigger to enable automated canary deployments.
push:
-# branches:
-# - 'main'
+ branches:
+ - 'main'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| # branches: | |
| # - 'main' | |
| branches: | |
| - 'main' |
| id: build-frontend | ||
| shell: bash | ||
| run: | ||
| docker build "." -f "docker-init/frontend/Dockerfile" -t "frontend-image:latest" --build-arg "API_BASE_URL=http://app:9000" |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Make API_BASE_URL configurable
Hardcoded API URL won't work across environments.
- docker build "." -f "docker-init/frontend/Dockerfile" -t "frontend-image:latest" --build-arg "API_BASE_URL=http://app:9000"
+ docker build "." -f "docker-init/frontend/Dockerfile" -t "frontend-image:latest" --build-arg "API_BASE_URL=${{secrets.API_BASE_URL}}"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| docker build "." -f "docker-init/frontend/Dockerfile" -t "frontend-image:latest" --build-arg "API_BASE_URL=http://app:9000" | |
| docker build "." -f "docker-init/frontend/Dockerfile" -t "frontend-image:latest" --build-arg "API_BASE_URL=${{secrets.API_BASE_URL}}" |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
.github/workflows/push_to_canary.yaml (1)
149-152: Improve image versioning strategyUsing static 'main' tag makes rollbacks difficult.
- IMAGE_TAG: main + IMAGE_TAG: canary-${{ github.run_number }}Also applies to: 161-170, 180-189, 213-222, 231-240, 249-258
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (1)
.github/workflows/push_to_canary.yaml(4 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/push_to_canary.yaml
54-54: shellcheck reported issue in this script: SC2015:info:2:11: Note that A && B || C is not if-then-else. C may run when A is true
(shellcheck)
🪛 yamllint (1.35.1)
.github/workflows/push_to_canary.yaml
[error] 50-50: trailing spaces
(trailing-spaces)
🔇 Additional comments (2)
.github/workflows/push_to_canary.yaml (2)
55-63: LGTM! Good error handling in SBT build steps
The error handling with proper exit codes and error messages is well implemented.
194-196: 🛠️ Refactor suggestion
Move GCP service account configuration to secrets
Service account details should be fully secured.
- project_id: ${{secrets.GCP_PROJECT_ID}}
- workload_identity_provider: projects/${{secrets.GCP_PROJECT_NUMBER}}/locations/global/workloadIdentityPools/github-actions/providers/github
- service_account: githubactions@${{secrets.GCP_PROJECT_ID}}.iam.gserviceaccount.com
+ project_id: ${{secrets.GCP_PROJECT_ID}}
+ workload_identity_provider: ${{secrets.GCP_WORKLOAD_IDENTITY_PROVIDER}}
+ service_account: ${{secrets.GCP_SERVICE_ACCOUNT}}Likely invalid or redundant comment.
| id: build-aws-app | ||
| shell: bash | ||
| run: | ||
| docker build "." -f "docker-init/Dockerfile" -t "aws-app-image:latest" --build-arg "AWS_DEFAULT_REGION=${{secrets.AWS_REGION}}" --build-arg "DYNAMO_ENDPOINT=https://dynamodb.${{secrets.AWS_REGION}}.amazonaws.com" --build-arg "JAVA_OPTS=-Xms16g -Xmx16g" --build-arg "SPARK_JAR=/app/cli/spark.jar" --build-arg "CLOUD_AWS_JAR=/app/cli/cloud_aws.jar" --build-arg "ONLINE_CLASS=ai.chronon.integrations.aws.AwsApiImpl" |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Move build arguments to secrets
Sensitive build arguments should be in secrets.
# For AWS App Image
- docker build "." -f "docker-init/Dockerfile" -t "aws-app-image:latest" --build-arg "AWS_DEFAULT_REGION=${{secrets.AWS_REGION}}" --build-arg "DYNAMO_ENDPOINT=https://dynamodb.${{secrets.AWS_REGION}}.amazonaws.com" --build-arg "JAVA_OPTS=-Xms16g -Xmx16g"
+ docker build "." -f "docker-init/Dockerfile" -t "aws-app-image:latest" --build-arg "AWS_DEFAULT_REGION=${{secrets.AWS_REGION}}" --build-arg "DYNAMO_ENDPOINT=${{secrets.DYNAMO_ENDPOINT}}" --build-arg "JAVA_OPTS=${{secrets.JAVA_OPTS}}"
# For GCP App Image
- docker build "." -f "docker-init/Dockerfile" -t "gcp-app-image:latest" --build-arg "GCP_DEFAULT_REGION=${{secrets.GCP_REGION}}" --build-arg "BIGTABLE_ENDPOINT=https://${{secrets.GCP_REGION}}-bigtable.googleapis.com" --build-arg "JAVA_OPTS=-Xms16g -Xmx16g"
+ docker build "." -f "docker-init/Dockerfile" -t "gcp-app-image:latest" --build-arg "GCP_DEFAULT_REGION=${{secrets.GCP_REGION}}" --build-arg "BIGTABLE_ENDPOINT=${{secrets.BIGTABLE_ENDPOINT}}" --build-arg "JAVA_OPTS=${{secrets.JAVA_OPTS}}"
# For Frontend Image
- docker build "." -f "docker-init/frontend/Dockerfile" -t "frontend-image:latest" --build-arg "API_BASE_URL=http://app:9000"
+ docker build "." -f "docker-init/frontend/Dockerfile" -t "frontend-image:latest" --build-arg "API_BASE_URL=${{secrets.API_BASE_URL}}"Also applies to: 120-120, 127-127
Comment on lines
80
to
82
| AWS_DEFAULT_REGION: ${{secrets.AWS_REGION}} | ||
| DYNAMO_ENDPOINT: https://dynamodb.${{secrets.AWS_REGION}}.amazonaws.com | ||
| JAVA_OPTS: "-Xms16g -Xmx16g" |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Move AWS configuration to secrets
Memory and endpoint settings should be configurable.
- AWS_DEFAULT_REGION: ${{secrets.AWS_REGION}}
- DYNAMO_ENDPOINT: https://dynamodb.${{secrets.AWS_REGION}}.amazonaws.com
- JAVA_OPTS: "-Xms16g -Xmx16g"
+ AWS_DEFAULT_REGION: ${{secrets.AWS_REGION}}
+ DYNAMO_ENDPOINT: ${{secrets.DYNAMO_ENDPOINT}}
+ JAVA_OPTS: ${{secrets.JAVA_OPTS}}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| AWS_DEFAULT_REGION: ${{secrets.AWS_REGION}} | |
| DYNAMO_ENDPOINT: https://dynamodb.${{secrets.AWS_REGION}}.amazonaws.com | |
| JAVA_OPTS: "-Xms16g -Xmx16g" | |
| AWS_DEFAULT_REGION: ${{secrets.AWS_REGION}} | |
| DYNAMO_ENDPOINT: ${{secrets.DYNAMO_ENDPOINT}} | |
| JAVA_OPTS: ${{secrets.JAVA_OPTS}} |
Comment on lines
13
to
18
| AWS_QUICKSTART_REPOSITORY: zipline-ai/quickstart | ||
| AWS_APP_REPOSITORY: zipline-ai/app | ||
| AWS_FRONTEND_REPOSITORY: zipline-ai/frontend | ||
| GAR_QUICKSTART_REPOSITORY: canary-images/quickstart | ||
| GAR_APP_REPOSITORY: canary-images/app | ||
| GAR_FRONTEND_REPOSITORY: canary-images/frontend |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Move repository paths to GitHub secrets
Repository paths should be configurable via secrets.
- AWS_QUICKSTART_REPOSITORY: zipline-ai/quickstart
- AWS_APP_REPOSITORY: zipline-ai/app
- AWS_FRONTEND_REPOSITORY: zipline-ai/frontend
- GAR_QUICKSTART_REPOSITORY: canary-images/quickstart
- GAR_APP_REPOSITORY: canary-images/app
- GAR_FRONTEND_REPOSITORY: canary-images/frontend
+ AWS_QUICKSTART_REPOSITORY: ${{ secrets.AWS_QUICKSTART_REPOSITORY }}
+ AWS_APP_REPOSITORY: ${{ secrets.AWS_APP_REPOSITORY }}
+ AWS_FRONTEND_REPOSITORY: ${{ secrets.AWS_FRONTEND_REPOSITORY }}
+ GAR_QUICKSTART_REPOSITORY: ${{ secrets.GAR_QUICKSTART_REPOSITORY }}
+ GAR_APP_REPOSITORY: ${{ secrets.GAR_APP_REPOSITORY }}
+ GAR_FRONTEND_REPOSITORY: ${{ secrets.GAR_FRONTEND_REPOSITORY }}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| AWS_QUICKSTART_REPOSITORY: zipline-ai/quickstart | |
| AWS_APP_REPOSITORY: zipline-ai/app | |
| AWS_FRONTEND_REPOSITORY: zipline-ai/frontend | |
| GAR_QUICKSTART_REPOSITORY: canary-images/quickstart | |
| GAR_APP_REPOSITORY: canary-images/app | |
| GAR_FRONTEND_REPOSITORY: canary-images/frontend | |
| AWS_QUICKSTART_REPOSITORY: ${{ secrets.AWS_QUICKSTART_REPOSITORY }} | |
| AWS_APP_REPOSITORY: ${{ secrets.AWS_APP_REPOSITORY }} | |
| AWS_FRONTEND_REPOSITORY: ${{ secrets.AWS_FRONTEND_REPOSITORY }} | |
| GAR_QUICKSTART_REPOSITORY: ${{ secrets.GAR_QUICKSTART_REPOSITORY }} | |
| GAR_APP_REPOSITORY: ${{ secrets.GAR_APP_REPOSITORY }} | |
| GAR_FRONTEND_REPOSITORY: ${{ secrets.GAR_FRONTEND_REPOSITORY }} |
Comment on lines
102
to
104
| GCP_DEFAULT_REGION: ${{secrets.GCP_REGION}} | ||
| BIGTABLE_ENDPOINT: https://${{secrets.GCP_REGION}}-bigtable.googleapis.com | ||
| JAVA_OPTS: "-Xms16g -Xmx16g" |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Move GCP configuration to secrets
Similar to AWS config, GCP settings should be configurable.
- GCP_DEFAULT_REGION: ${{secrets.GCP_REGION}}
- BIGTABLE_ENDPOINT: https://${{secrets.GCP_REGION}}-bigtable.googleapis.com
- JAVA_OPTS: "-Xms16g -Xmx16g"
+ GCP_DEFAULT_REGION: ${{secrets.GCP_REGION}}
+ BIGTABLE_ENDPOINT: ${{secrets.BIGTABLE_ENDPOINT}}
+ JAVA_OPTS: ${{secrets.JAVA_OPTS}}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| GCP_DEFAULT_REGION: ${{secrets.GCP_REGION}} | |
| BIGTABLE_ENDPOINT: https://${{secrets.GCP_REGION}}-bigtable.googleapis.com | |
| JAVA_OPTS: "-Xms16g -Xmx16g" | |
| GCP_DEFAULT_REGION: ${{secrets.GCP_REGION}} | |
| BIGTABLE_ENDPOINT: ${{secrets.BIGTABLE_ENDPOINT}} | |
| JAVA_OPTS: ${{secrets.JAVA_OPTS}} |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This creates a Github workflow which pushes new versions of the containers to AWS and GCP for use as a canary.
This workflow installs Thrift, runs sbt assembly, and builds the images. It then pushes to AWS ECR and GCP Artifact Registry
Checklist
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Chores
.gitignoreto include new entries for generated credentials and specific data paths.