build(actions): AS-771 add package build #6152
Conversation
WalkthroughA new Changes
Sequence Diagram(s)sequenceDiagram
participant GitHub Actions
participant build-app (reusable workflow)
participant build-image
GitHub Actions->>build-app (reusable workflow): Trigger build-app job
build-app (reusable workflow)-->>GitHub Actions: Produce dist artifact
GitHub Actions->>build-image: Trigger build-image job (needs: build-app)
build-image->>build-image: Download dist artifact
build-image->>build-image: Build Docker image with BUILD_TYPE=source
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/build-docker-internal.yml (1)
13-13: Consider adding explicit failure-propagation.
needs: [build-app]enforces ordering but not status checks.
To prevent the image build from running on a failed app build, add:if: needs.build-app.result == 'success'This keeps minutes down and avoids misleading partial-success runs.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/build-docker-internal.yml(3 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: swheaton
PR: voxel51/fiftyone#0
File: :0-0
Timestamp: 2024-09-20T14:09:31.848Z
Learning: Ensure that code review comments on `fiftyone/core/cli.py` are accurate and relevant to the actual code changes.
Learnt from: swheaton
PR: voxel51/fiftyone#0
File: :0-0
Timestamp: 2024-10-17T01:03:25.080Z
Learning: Ensure that code review comments on `fiftyone/core/cli.py` are accurate and relevant to the actual code changes.
.github/workflows/build-docker-internal.yml (1)
Learnt from: kevin-dimichel
PR: voxel51/fiftyone#5173
File: .github/workflows/build-docs.yml:54-54
Timestamp: 2024-11-21T20:59:53.855Z
Learning: The `RO_FOT_FG_PAT` token used in `.github/workflows/build-docs.yml` has the necessary permissions to checkout the `voxel51/fiftyone-teams` repository.
🔇 Additional comments (3)
.github/workflows/build-docker-internal.yml (3)
8-10: Verify thatbuild.ymlis declared as aworkflow_call.The reusable workflow reference (
uses: ./.github/workflows/build.yml) will only resolve ifbuild.ymlcontains:on: workflow_call:Without it, this job will fail at runtime with the “no callable workflow” error.
29-33: Potential double-nesting of thedistdirectory.
actions/download-artifactextracts the artifact contents directly intopath.
If the uploaded artifact itself is already adist/folder, the resulting layout will bedist/dist/*, breaking any later COPY commands in your Dockerfile that expectdist/*.Confirm the artifact structure, or defensively flatten it:
- name: Download dist uses: actions/download-artifact@v4 with: name: dist path: .
59-59: Change toBUILD_TYPE=sourcealters the Dockerfile contract.Double-check that every Dockerfile stage and conditional logic referenced by
BUILD_TYPEsupports the newsourcevalue; otherwise the build will silently fall back to defaults or error out late in the process.
What changes are proposed in this pull request?
I recently merged #6144. Locally, it worked because I had cached/built artifacts. In the real world, we need to build the app first > download the distribution > build from source.
This PR makes those fixes.
How is this patch tested? If it is not, please explain why.
Verified successful action
Release Notes
Is this a user-facing change that should be mentioned in the release notes?
notes for FiftyOne users.
(Details in 1-2 sentences. You can just refer to another PR with a description
if this PR is part of a larger change.)
What areas of FiftyOne does this PR affect?
fiftyonePython library changesSummary by CodeRabbit