Skip to content

Conversation

vsoch
Copy link
Contributor

@vsoch vsoch commented Jul 7, 2023

Problem: we cannot currently install the python mpijob module
Solution: add a setup.py proper that is ignored by the generator

Notes:

  • I chose kubeflow-mpijob to reflect the name of the Python module (typical practice, with a prefix)
  • Tested with pip install .
  • I didn't test with the generator yet, but saw this pattern to add the file to be ignored that (hopefully) will work.

Future improvements:

  • more documentation about install
  • full examples directory
  • release package to pypi

This will close #578

Copy link
Member

@tenzen-y tenzen-y left a comment

Choose a reason for hiding this comment

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

@vandanavk Thanks for the great contribution!

"""
setuptools.setup(
name="kubeflow-mpijob",
version="0.0.1",
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
version="0.0.1",
version="0.4.0",

Should we use the release version?

name="kubeflow-mpijob",
version="0.0.1",
author="Kubeflow Authors",
author_email="wg-batch@kubernetes.io",
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
author_email="wg-batch@kubernetes.io",
author_email="kubeflow-discuss@googlegroups.com",

Kubernetes WG batch doesn't maintain mpi-operator. So we should set another one to here.
@terrytangyuan @alculquicondor What do you think about using kubeflow-discuss@googlegroups.com?

Copy link
Member

Choose a reason for hiding this comment

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

It’s better not to spam that mailing list with bug reports

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I usually put a faux GitHub email here - the <name>@users.noreply.github.com but if someone has an actual email, that is likely ideal. I'm not sure how often these emails get used, but I would say if the group is protected by someone needing to join, that would get around bots that scrape the emails (and spam) and maybe even be a good approach? Someone that wants help with the package should maybe reach out to this list?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay I updated to kubeflow@users.noreply.github.com. it is definitely useless and fake, but will satisfy the pypi requirements if you ever decide to release there (you just need the contact field defined with something resembling an email).

Copy link
Member

Choose a reason for hiding this comment

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

No objection.

Copy link
Member

Choose a reason for hiding this comment

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

sgtm

Copy link
Contributor

Choose a reason for hiding this comment

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

sgtm

Problem: we cannot currently install the python mpijob module
Solution: add a setup.py proper that is ignored by the generator
Signed-off-by: vsoch <vsoch@users.noreply.github.com>
Copy link
Member

@terrytangyuan terrytangyuan left a comment

Choose a reason for hiding this comment

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

Thanks!

/lgtm

@google-oss-prow
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: terrytangyuan

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@google-oss-prow google-oss-prow bot merged commit 18250f5 into kubeflow:master Jul 10, 2023
@cliu-nuro
Copy link

Hi @vsoch , what's the timeline like to release this package to pypi?

@vsoch
Copy link
Contributor Author

vsoch commented Jul 25, 2023

hey @cliu-nuro - there is no timeline, it's more that we could do it when it's requested. It isn't super hard, a maintainer/owner would need to create the first (dummy) deployment locally, usually you just do:

python setup.py sdist
twine upload dist/<path-to-tar-gz>

or create the package manually in the UI. Then get a pypi token / username for the package, explicitly, and then you just need
a workflow like this:

name: release mpijob

on:
  release:
    types: [created]

jobs:
  deploy:
    runs-on: ubuntu-latest

    steps:
    - uses: actions/checkout@v3

    - name: Install
      run: conda create --quiet --name mpijob twine

    - name: Install dependencies
      run: |
        export PATH="/usr/share/miniconda/bin:$PATH"
        source activate mpijob
        cd /path/to/where/mpijob/setup.py/is
        pip install -e .[all]
        pip install setuptools wheel twine
    - name: Build and publish
      env:
        TWINE_USERNAME: ${{ secrets.PYPI_USER }}
        TWINE_PASSWORD: ${{ secrets.PYPI_PASS }}
      run: |
        export PATH="/usr/share/miniconda/bin:$PATH"
        source activate mpijob
        python setup.py sdist bdist_wheel
        twine upload dist/*

And that should upload on releases. I'm not a maintainer here so I can't do most of that, but happy to provide guidance!

@vsoch vsoch deleted the add/setup-py branch July 25, 2023 01:11
@alculquicondor
Copy link
Contributor

@terrytangyuan what was the process to release to pypi?

@terrytangyuan
Copy link
Member

I don't think we have a process yet. It's not published to PyPI.

@terrytangyuan
Copy link
Member

We can instruct users to install directly from GitHub.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

python setup.py doesn't appear to install?
5 participants