Skip to content

Conversation

@porkpink
Copy link
Contributor

@porkpink porkpink commented Aug 5, 2025

🔨 Change deployment from serial to parallel to improve efficiency.

Copilot AI review requested due to automatic review settings August 5, 2025 13:32
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the GitHub Actions workflow to improve deployment efficiency by changing from serial to parallel build execution. The workflow consolidates three separate build jobs into a single parallelized job using a matrix strategy.

  • Consolidates three separate build jobs (build-main, build-data-process, build-web) into one build-images job with matrix strategy
  • Implements conditional logic for data-process specific model caching operations
  • Updates deployment job dependency to reference the single build job

Comment on lines 58 to 59
else
docker build --build-arg MIRROR=https://registry.npmmirror.com -t nexent/nexent-web -f make/web/Dockerfile .
Copy link

Copilot AI Aug 5, 2025

Choose a reason for hiding this comment

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

The else clause assumes any target other than 'main' or 'data-process' is 'web'. This could lead to unexpected behavior if the matrix is extended with additional targets. Consider explicitly checking for 'web' target: elif [ "${{ matrix.target }}" = "web" ]; then

Suggested change
else
docker build --build-arg MIRROR=https://registry.npmmirror.com -t nexent/nexent-web -f make/web/Dockerfile .
elif [ "${{ matrix.target }}" = "web" ]; then
docker build --build-arg MIRROR=https://registry.npmmirror.com -t nexent/nexent-web -f make/web/Dockerfile .
else
echo "Unknown target: ${{ matrix.target }}" >&2
exit 1

Copilot uses AI. Check for mistakes.
- name: Clone model if not cached
if: steps.check-model.outputs.cache-hit == 'false'
if: matrix.target == 'data-process' && steps.check-model.outputs.cache-hit == 'false'
Copy link

Copilot AI Aug 5, 2025

Choose a reason for hiding this comment

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

The condition references steps.check-model.outputs.cache-hit but the check-model step only runs when matrix.target == 'data-process'. For other matrix targets, this step doesn't exist and the condition will fail. Consider restructuring the conditions or using a default value.

Suggested change
if: matrix.target == 'data-process' && steps.check-model.outputs.cache-hit == 'false'
if: ${{ matrix.target == 'data-process' && steps.check-model.outputs.cache-hit == 'false' }}

Copilot uses AI. Check for mistakes.
@codecov
Copy link

codecov bot commented Aug 5, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@porkpink porkpink changed the title 🔨 Change deployment from serial to parallel to improve efficiency. [WIP]🔨 Change deployment from serial to parallel to improve efficiency. Aug 5, 2025
@porkpink porkpink closed this Aug 7, 2025
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.

2 participants