Skip to content
This repository was archived by the owner on Oct 11, 2024. It is now read-only.

Generate tarball along with wheel build, and upload both in a package to GH #138

Merged
merged 3 commits into from
Mar 28, 2024
Merged

Generate tarball along with wheel build, and upload both in a package to GH #138

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
cf44ca5
rebase
Mar 25, 2024
e913b03
don't repeat wheel generation
Mar 25, 2024
ffa3c7d
address comment
Mar 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 15 additions & 1 deletion .github/actions/nm-build-vllm-whl/action.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,12 @@ outputs:
whl:
description: 'basename for generated whl'
value: ${{ steps.whl.outputs.whl }}
tarball:
description: 'basename for generated tarball'
value: ${{ steps.whl.outputs.tarball }}
artifact:
description: 'artifact name'
value: ${{ steps.whl.outputs.artifact }}
runs:
using: composite
steps:
Expand All @@ -24,11 +30,19 @@ runs:
source $(pyenv root)/versions/${{ inputs.python }}/envs/${VENV}/bin/activate
SUCCESS=0
pip3 wheel --no-deps -w dist . || SUCCESS=$?
echo "status=${SUCCESS}" >> "$GITHUB_OUTPUT"
ls -alh dist/
BASE=$(./.github/scripts/convert-version ${{ inputs.python }})
WHL_FILEPATH=$(find dist -iname "*nm_vllm*${BASE}*.whl")
WHL=$(basename ${WHL_FILEPATH})
echo "whl=${WHL}" >> "$GITHUB_OUTPUT"
# generate .tar.gz
$SUCCESS || python3 setup.py sdist || SUCCESS=$?
Copy link

Choose a reason for hiding this comment

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

Hey Dan. The $SUCCESS || in $SUCCESS || python3 setup.py sdist || SUCCESS=$? isn't doing what is intended. I see

/opt/actions-runner/_work/_temp/68fcf277-8944-46d0-8362-4c8020852b3b.sh: line 12: 0: command not found

in https://github.com/neuralmagic/nm-vllm/actions/runs/8359879522/job/22884186408
I think it is safe to remove.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hi Varun, the $SUCCESS in $SUCCESS || python3 setup.py sdist || SUCCESS=$? is to keep the value in SUCCESS if the previous wheel generation step failed. It'll only run the tarball generation only if $SUCCESS=0, i.e. the wheel generation step is successful.

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated command so it'll work as intended, good catch.

Copy link

Choose a reason for hiding this comment

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

Thanks Dan.

ls -alh dist/
Copy link

Choose a reason for hiding this comment

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

Not sure if this is intentional. Using the same dist folder includes the wheel in the tarball. May be use a different directory if it isn't intentional ? @mgoin

Copy link
Member Author

Choose a reason for hiding this comment

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

The sdist argument is to generate source distribution, i.e. the tar.gz file. It'll be generated under the same folder as the wheel which is generated in an earlier step, i.e. dist/. This is the default output folder for generated packages. We want to keep them under dist/ since we'll upload them both to GH in one step by just providing the dist/ path to an upload action.

TARBALL_FILEPATH=$(find dist -iname "*nm-vllm*.tar.gz")
TARBALL=$(basename ${TARBALL_FILEPATH})
echo "status=${SUCCESS}" >> "$GITHUB_OUTPUT"
echo "tarball=${TARBALL}" >> "$GITHUB_OUTPUT"
ARTIFACT=`echo "${WHL}" | sed 's/.whl//g'`
echo "artifact=${ARTIFACT}" >> "$GITHUB_OUTPUT"
exit ${SUCCESS}
shell: bash
4 changes: 4 additions & 0 deletions .github/actions/nm-whl-summary/action.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ inputs:
whl:
description: 'whl file tested'
required: true
tarball:
description: 'tar.gz file tested'
required: true
runs:
using: composite
steps:
Expand All @@ -34,4 +37,5 @@ runs:
echo "| branch name: | '${{ github.ref_name }}' |" >> $GITHUB_STEP_SUMMARY
echo "| python: | ${{ inputs.python }} |" >> $GITHUB_STEP_SUMMARY
echo "| whl: | ${{ inputs.whl }} |" >> $GITHUB_STEP_SUMMARY
echo "| tarball: | ${{ inputs.tarball }} |" >> $GITHUB_STEP_SUMMARY
shell: bash
7 changes: 4 additions & 3 deletions .github/workflows/build-whl.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,12 +113,12 @@ jobs:
python: ${{ inputs.python }}
venv: ${{ env.VENV_BUILD_BASE }}

- name: upload whl
- name: upload dist
uses: actions/upload-artifact@v4
if: success() || failure()
with:
name: ${{ steps.build_whl.outputs.whl }}
path: dist/${{ steps.build_whl.outputs.whl }}
name: ${{ steps.build_whl.outputs.artifact }}
path: dist/
retention-days: 15

- name: summary
Expand All @@ -129,6 +129,7 @@ jobs:
testmo_run_url: https://neuralmagic.testmo.net/automation/runs/view/${{ steps.create_testmo_run.outputs.id }}
python: ${{ inputs.python }}
whl: ${{ steps.build_whl.outputs.whl }}
tarball: ${{ steps.build_whl.outputs.tarball }}

- name: complete testmo run
uses: ./.github/actions/nm-testmo-run-complete/
Expand Down
Loading