Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Workflow for pushing conda nightly binaries and merging all nightly runs to a single workflow #1685

Merged
merged 30 commits into from
Jul 14, 2022

Conversation

agunapal
Copy link
Collaborator

@agunapal agunapal commented Jun 13, 2022

Description

Merged torchserve-nightly, torch-model-archiver-nightly, torch-workflow-archiver and torchserve conda nightly
into 1 single flow.

Current Behavior:

  • We have a separate workflow for each of PyPI builds of torchserve-nightly, torch-workflow-archiver-nightly and torch-model-archiever-nightly
  • No workflow for conda nightly binaries

Problem:

  • Unnecessary use of extra compute

Solution:

The workflow does the following:

  • Launches binaries creation workflow for the 3 OS: ubtunu, macOS and windows
  • Creates a conda environment
  • Builds and uploads the PyPI binaries (serve, model-archiver and workflow-archiver) for ubuntu, but skips it for windows and macOS because the binaries have the same name
  • Builds and uploads conda binaries for all the 3 OS

Miscellaneous:

  • Introduced a new secret key called CONDA_NIGHTLY_TOKEN to push the conda nightly binaries to https://anaconda.org/pytorch-nightly/
  • Wrote a new function called build_dist_whl to build wheel files. Build nightly wheel files by passing "nightly" arg
  • Updated upload_pypi_packages to upload to test.pypi.org or pypi.org based on "test-pypi" arg
  • Update upload_conda_packages to upload based on setting ANACONDA_API_TOKEN with CONDA_NIGHTLY_TOKEN

Fixes #(issue)

Reduces developer time to build and push binaries

Type of change

  • New feature (non-breaking change which adds functionality)

Feature/Issue validation/testing

  • Test A

Screen Shot 2022-07-12 at 12 29 32 PM

Screen Shot 2022-07-12 at 12 29 01 PM

Screen Shot 2022-07-12 at 12 44 31 PM

Screen Shot 2022-07-12 at 12 43 53 PM

  • Test D
    Tested the binary on GPU as well

Screen Shot 2022-06-17 at 5 21 03 PM

  • Test E
    PyPI binaries test

Screen Shot 2022-07-12 at 11 56 31 AM

  • Test F
    PyPI binaries test

Screen Shot 2022-07-12 at 12 06 05 PM

Screen Shot 2022-07-12 at 12 06 47 PM

Checklist:

  • [ Yes] Did you have fun?
  • [ Yes] Have you added tests that prove your fix is effective or that this feature works?
  • [ Yes] Has code been commented, particularly in hard-to-understand areas?
  • [ Yes] Have you made corresponding changes to the documentation?

@agunapal agunapal requested a review from msaroufim June 13, 2022 20:53
@maaquib maaquib requested a review from lxning June 13, 2022 21:54
@maaquib maaquib added the enhancement New feature or request label Jun 13, 2022
@msaroufim
Copy link
Member

msaroufim commented Jun 13, 2022

Thanks @ankithagunapal! I'm super glad to see this change. There's a few more things we need to do to merge this

  1. The package should be called *-nightly e.g torchserve-nightly to clearly differentiate it from the official torchserve releases and should be part of the pytorch-nightly org https://anaconda.org/pytorch-nightly
  2. The name of the package needs to include the date for example see what torchvision does https://anaconda.org/pytorch-nightly/torchvision/files. In our case let's use the same naming convention we use in our pypi nightly scripts to stay consistent https://github.com/pytorch/serve/blob/master/setup.py#L56 which will show up as in https://pypi.org/project/torchserve-nightly/#description
  3. There's already a secret called CONDA_PASSWORD which you can leverage (I can rename it too if you think I didn't give it the correct name), the permissions there are fairly broad so feel to ping Eli Uriegas internally or just ping me again when I'm back from PTO in case you're not sure if we're doing something destructive. The last thing we want to do is override official releases or delete stuff

@msaroufim msaroufim added the p1 mid priority label Jun 15, 2022
@msaroufim msaroufim added this to the v0.6.1 milestone Jun 15, 2022
@msaroufim msaroufim added this to in review in v0.6.1 lifecycle Jun 15, 2022
@msaroufim msaroufim linked an issue Jun 15, 2022 that may be closed by this pull request
@agunapal
Copy link
Collaborator Author

@msaroufim Have updated the binaries to have a format (including date) which is consistent with other packages in pytorch.

Updated yml file to use CONDA_NIGHTLY_TOKEN which will push the binaries to https://anaconda.org/pytorch-nightly/

Copy link
Member

@msaroufim msaroufim left a comment

Choose a reason for hiding this comment

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

LGTM, some minor nits are left that that need to be fixed before merge

Also I'd like us to add instructions on the main README for installation for both regular and nightly conda https://github.com/pytorch/serve#-quick-start-with-torchserve

binaries/conda/build_packages.py Outdated Show resolved Hide resolved
@lxning
Copy link
Collaborator

lxning commented Jun 20, 2022

@agunapal conda binary is based on pypi. We already have pypi nightly build. To make pypi and conda binary consistent, I think they should be sit together; otherwise, it would potentially cause binary discrepancy and also waste resource. In other words, the existing pypi nightly build workflow is extended to add one more steps to build/release conda.

@msaroufim
Copy link
Member

@agunapal conda binary is based on pypi. We already have pypi nightly build. To make pypi and conda binary consistent, I think they should be sit together; otherwise, it would potentially cause binary discrepancy and also waste resource. In other words, the existing pypi nightly build workflow is extended to add one more steps to build/release conda.

That's a good point, I think we can actually unify the 3 actions we have for nightly torchserve, workflow-archiver and model-archiver into a single action and then add a step there to also do conda https://github.com/pytorch/serve/blob/master/.github/workflows/torchserve-nightly-build.yml

@agunapal
Copy link
Collaborator Author

@agunapal conda binary is based on pypi. We already have pypi nightly build. To make pypi and conda binary consistent, I think they should be sit together; otherwise, it would potentially cause binary discrepancy and also waste resource. In other words, the existing pypi nightly build workflow is extended to add one more steps to build/release conda.

Thanks @lxning Thats a good suggestion

@agunapal agunapal changed the title Workflow for pushing conda nightly cpu binaries Workflow for pushing conda nightly binaries and merging all nightly runs to a single workflow Jun 24, 2022
@agunapal agunapal requested a review from msaroufim June 24, 2022 21:12
Copy link
Collaborator Author

@agunapal agunapal left a comment

Choose a reason for hiding this comment

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

4 workflows have been merged to a single workflow

ts_scripts/install_dependencies.py Outdated Show resolved Hide resolved
@agunapal agunapal requested a review from msaroufim June 29, 2022 18:02
binaries/build.py Outdated Show resolved Hide resolved
binaries/build.py Outdated Show resolved Hide resolved
binaries/build.py Outdated Show resolved Hide resolved
binaries/build.py Outdated Show resolved Hide resolved
binaries/build.py Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
binaries/upload.py Outdated Show resolved Hide resolved
Copy link
Member

@msaroufim msaroufim left a comment

Choose a reason for hiding this comment

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

Approved, let's just add some pypi binaries uploaded

README.md Show resolved Hide resolved
binaries/build.py Outdated Show resolved Hide resolved
.github/workflows/torchserve-nightly-build.yml Outdated Show resolved Hide resolved
binaries/build.py Show resolved Hide resolved
binaries/upload.py Show resolved Hide resolved
binaries/upload.py Show resolved Hide resolved
.github/workflows/torchserve-nightly-build.yml Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request p1 mid priority
Projects
Development

Successfully merging this pull request may close these issues.

Nightly conda builds
5 participants