Skip to content

Build Wheel Action#105

Merged
gkumbhat merged 3 commits intofoundation-model-stack:mainfrom
jbusche:jb-wheel-issue57
Apr 13, 2024
Merged

Build Wheel Action#105
gkumbhat merged 3 commits intofoundation-model-stack:mainfrom
jbusche:jb-wheel-issue57

Conversation

@jbusche
Copy link
Copy Markdown
Collaborator

@jbusche jbusche commented Apr 2, 2024

Description of the change

This PR adds build and publish Github Actions that will allow extra testing of the build and wheel files with tox and then ultimately publish the library out on the production PyPI when we get the token figured out.

Related issue number

Closes issue 671 and external issue #57

How to verify the PR

From my github repo, I synced my branch with main and then am able to manually run the GHA successfully, giving it the input version, for example 1.0.dev3 in this run: https://github.com/jbusche/fms-hf-tuning/actions/runs/8547067789

I tried then installing from a new python env:

pip install fms-hf-tuning==1.0.dev3
pip install fms-hf-tuning[dev]==1.0.dev3
pip list |grep fms
fms-hf-tuning            1.0.dev3

Was the PR tested

Yes, by running the GHA in my fork. (The GHA won't run in the upstream repo until it's synced with main branch)

  • I have added >=1 unit test(s) for every new method I have added.
  • I have ensured all unit tests pass

Comment thread .github/workflows/build-library.yml
@jbusche
Copy link
Copy Markdown
Collaborator Author

jbusche commented Apr 2, 2024

/retest

Comment thread .github/workflows/publish-library.yaml Outdated
Comment thread .github/workflows/publish-library.yaml Outdated
@jbusche jbusche force-pushed the jb-wheel-issue57 branch 2 times, most recently from 9847340 to e659341 Compare April 2, 2024 22:13
Comment thread .github/workflows/build-and-publish.yaml
@jbusche jbusche force-pushed the jb-wheel-issue57 branch from 7a6bede to 6a83db9 Compare April 3, 2024 23:57
@jbusche jbusche marked this pull request as ready for review April 4, 2024 00:06
@jbusche
Copy link
Copy Markdown
Collaborator Author

jbusche commented Apr 4, 2024

OK this GHA works, building the wheel file and pushing up to PyPI.

Some questions remain:

  1. Is manually running the GHA and giving it the version number the right way to publish to PyPI or is it wanted to be automatic, for example when branch name = release or tag or something? If so, how do we want to increment the version number... be sure to update the toml file prior to pushing the release branch, or maybe from an individual version file?
  2. With the trusted upload to pypi, there is an "environment name" option. (see doc) It's recommended to set for extra security, but I don't know what the environment name would be or how to set that.
  3. In testing pushed 0.0.1 already, didn't realize it's not deletable. I need to research if, while not deletable if it's updatable with newer wheel file(s) on the same version.

@tedhtchang
Copy link
Copy Markdown
Collaborator

@z103cb Please advice.

Copy link
Copy Markdown

@z103cb z103cb left a comment

Choose a reason for hiding this comment

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

I think that this PR is ok. We should consider perhaps having this triggered from "Create release" page in GITHUB. An example from RH library I have collaborated on can be found here: https://github.com/opendatahub-io/caikit-nlp-client/blob/main/.github/workflows/release.yml

Comment on lines +43 to +46
- name: Replace release-version with provided input value
run: |
export RELEASE_VERSION=${{ github.event.inputs.release-version }}
sed -i "s/^version = \"[0-9.]*\"$/version = \"${RELEASE_VERSION}\"/g" pyproject.toml
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Perhaps we should commit this change to the file or conversely, do it in the way, it's done the here.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

i could not find version = is matched in pyproject.toml

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

you may find version = in this pyproject.toml

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

if we go with git release, then I think we won't need to do this sed here ? (since we would do it on tags)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

OK - working on this... thanks for the advice, reviews!

@z103cb
Copy link
Copy Markdown

z103cb commented Apr 4, 2024

OK this GHA works, building the wheel file and pushing up to PyPI.

Some questions remain:

  1. Is manually running the GHA and giving it the version number the right way to publish to PyPI or is it wanted to be automatic, for example when branch name = release or tag or something? If so, how do we want to increment the version number... be sure to update the toml file prior to pushing the release branch, or maybe from an individual version file?

I would prefer it be done automatically, when a new "GitHub release" is created. In the example I have provided, believe that the version is derived from the tag on the release. The relevant bits are in:

[build-system]
build-backend = "setuptools.build_meta"
requires = [
    "setuptools>=60",
    "setuptools-scm>=8.0",
]


[project]
name = "caikit_nlp_client"
dynamic = ["version"]
description = "caikit-nlp client"
license = { text = "Apache License 2.0" }
readme = "README.md"
classifiers=[
    "License :: OSI Approved :: Apache Software License",
    "Development Status :: 4 - Beta",
    "Programming Language :: Python :: 3",
    "Programming Language :: Python :: 3.9",
    "Programming Language :: Python :: 3.10",
    "Programming Language :: Python :: 3.11",
]
  1. With the trusted upload to pypi, there is an "environment name" option. (see doc) It's recommended to set for extra security, but I don't know what the environment name would be or how to set that.

I would ask the owners of the repo

  1. In testing pushed 0.0.1 already, didn't realize it's not deletable. I need to research if, while not deletable if it's updatable with newer wheel file(s) on the same version.

I would not worry too much to about it.

Copy link
Copy Markdown

@zdtsw zdtsw left a comment

Choose a reason for hiding this comment

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

some nitty gritty for the GH action part

Comment thread .github/workflows/build-and-publish.yaml Outdated
Comment thread .github/workflows/build-and-publish.yaml Outdated
Comment on lines +43 to +46
- name: Replace release-version with provided input value
run: |
export RELEASE_VERSION=${{ github.event.inputs.release-version }}
sed -i "s/^version = \"[0-9.]*\"$/version = \"${RELEASE_VERSION}\"/g" pyproject.toml
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

i could not find version = is matched in pyproject.toml

@tedhtchang
Copy link
Copy Markdown
Collaborator

tedhtchang commented Apr 4, 2024

I would prefer it be done automatically, when a new "GitHub release" is created. In the example I have provided, believe that the version is derived from the tag on the release.

Update @jbusche I'v tested with those changes in a repo. I believe these are changes you need in
pyproject.toml and the build-and-push.yaml

[build-system]
requires = [
    "setuptools>=60",
    "setuptools-scm>=8.0"]

[project]
dynamic = ["version"]

[tool.setuptools_scm]
version_file = "tuning/_version.py"

This example would publish the package with 0.0.1rc-2 release in pypi
image

Update: The setuptools_scm actually writes to the _version.py file.
@z103cb This a good suggestion. Btw, the version comes from a file but I am not sure how the file got created.

Comment thread .github/workflows/build-and-publish.yaml
Comment thread .github/workflows/build-and-publish.yaml Outdated
Comment thread .github/workflows/build-and-publish.yaml Outdated
Comment thread .github/workflows/build-and-publish.yaml Outdated
Comment thread .github/workflows/build-and-publish.yaml Outdated
Comment on lines +43 to +46
- name: Replace release-version with provided input value
run: |
export RELEASE_VERSION=${{ github.event.inputs.release-version }}
sed -i "s/^version = \"[0-9.]*\"$/version = \"${RELEASE_VERSION}\"/g" pyproject.toml
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

if we go with git release, then I think we won't need to do this sed here ? (since we would do it on tags)

Comment thread .github/workflows/build-and-publish.yaml
Comment thread .github/workflows/build-and-publish.yaml Outdated
@jbusche jbusche force-pushed the jb-wheel-issue57 branch from 2cd7ba4 to 5e6f1a5 Compare April 9, 2024 17:21
@jbusche
Copy link
Copy Markdown
Collaborator Author

jbusche commented Apr 9, 2024

@gkumbhat and @tedhtchang, thanks for the suggestions, I got it to work on release where it builds the wheel and publishes up to PiPY. I just tried with 0.0.1rc.5 from my repo (had to use my main bracn so it would work):
https://github.com/jbusche/fms-hf-tuning/actions/runs/8620146700/job/23626417173

pip install fms-hf-tuning==0.0.1rc.5
pip install fms-hf-tuning[dev]==0.0.1rc.5

I think the only thing perhaps left is that people didn't want it set to just python 3.9, so I added a matrix that added all three in a manor like I saw in caitkit-nlp However having 3.9, 3.10 and 3.11 ended up launching three identical workflows which isn't what I wanted, so I've made it just 3.11. Is that sufficient?

@jbusche jbusche force-pushed the jb-wheel-issue57 branch 2 times, most recently from d051df3 to 4ccd221 Compare April 10, 2024 20:19
Comment thread CONTRIBUTING.md
Comment thread tox.ini
jbusche added 3 commits April 11, 2024 15:43
Signed-off-by: James Busche <jbusche@us.ibm.com>

release version in build workflow

Signed-off-by: James Busche <jbusche@us.ibm.com>

release version in build workflow

Signed-off-by: James Busche <jbusche@us.ibm.com>

fix build workflow for version

Signed-off-by: James Busche <jbusche@us.ibm.com>

space fix in build workflow for version

Signed-off-by: James Busche <jbusche@us.ibm.com>

Add publish to pypi

Signed-off-by: James Busche <jbusche@us.ibm.com>

condense publish to pypi

Signed-off-by: James Busche <jbusche@us.ibm.com>

cleanup build yaml

Signed-off-by: James Busche <jbusche@us.ibm.com>

Switch publish to release or tag

Signed-off-by: James Busche <jbusche@us.ibm.com>

adding py311 and fix dependencies

Signed-off-by: James Busche <jbusche@us.ibm.com>

remove version from setup.py and add debugging

Signed-off-by: James Busche <jbusche@us.ibm.com>

adding PiPY publish

Signed-off-by: James Busche <jbusche@us.ibm.com>
Signed-off-by: James Busche <jbusche@us.ibm.com>
Signed-off-by: James Busche <jbusche@us.ibm.com>
@jbusche
Copy link
Copy Markdown
Collaborator Author

jbusche commented Apr 11, 2024

PR 105 is looking good in testing from my main branch, testing creating a new release. It builds the wheel now without source (thanks for the tip @ted CHANG on how to only build the wheel file) and publishes up to PiPY. Can see that only the wheel file is published: https://pypi.org/project/fms-hf-tuning/0.0.1rc6/#files

Tried on a fresh python environment, and it's looking good:

pip install --upgrade pip
pip install fms-hf-tuning==0.0.1rc.6
pip install fms-hf-tuning[dev]==0.0.1rc.6

and the install looks good:

pip list |grep fms
fms-hf-tuning            0.0.1rc6

and I was successfully able to run the twitter_complaints test

100%|██████████████████████████████████████████████████████████████████████████████████████████| 3/3 [01:33<00:00, 31.08s/it]

Copy link
Copy Markdown
Collaborator

@tedhtchang tedhtchang left a comment

Choose a reason for hiding this comment

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

/LGTM Thanks.

Comment thread setup.py
Comment thread pyproject.toml
@gkumbhat gkumbhat merged commit b747c5f into foundation-model-stack:main Apr 13, 2024
@jbusche jbusche deleted the jb-wheel-issue57 branch April 15, 2024 17:15
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.

5 participants