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

Add pyproject.toml and cleanup sdk installation #223

Merged
merged 5 commits into from
Jul 6, 2023

Conversation

kannon92
Copy link
Contributor

@kannon92 kannon92 commented Jul 6, 2023

This PR adds pyproject.toml instead of setup.py. The python community is moving to adopt pyproject.toml. Another reason I moved to this is that the tool chain we use to generate the python code has an invalid setup.py.

You can install our package like python3 -m pip install sdk/python/. with the pyproject.toml file.

@danielvegamyhre this should fix your issue with /tmp and egg files.

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jul 6, 2023
@kannon92
Copy link
Contributor Author

kannon92 commented Jul 6, 2023

@ahg-g and @danielvegamyhre IMO we should maybe consider this as a patch release to 0.2.0. 0.2.0 package isn't capable of being installed as a package in a virtual environment.

@danielvegamyhre
Copy link
Contributor

@ahg-g and @danielvegamyhre IMO we should maybe consider this as a patch release to 0.2.0. 0.2.0 package isn't capable of being installed as a package in a virtual environment.

Agreed, we can include this in a v0.2.1 release. Thanks for putting this together!

@danielvegamyhre
Copy link
Contributor

@kannon92 I checked out this PR locally and tried to install the jobset package in my virtualenv and encountered this error:

~/go/src/sigs.k8s.io/jobset$ python3 -m pip install sdk/python/.
Processing ./sdk/python
  Installing build dependencies ... done
  Getting requirements to build wheel ... error
  error: subprocess-exited-with-error
  
  × Getting requirements to build wheel did not run successfully.
  │ exit code: 1
  ╰─> [14 lines of output]
      error: Multiple top-level packages discovered in a flat-layout: ['lib', 'jobset', 'include'].
      
      To avoid accidental inclusion of unwanted files or directories,
      setuptools will not proceed with this build.
      
      If you are trying to create a single distribution with multiple packages
      on purpose, you should not rely on automatic discovery.
      Instead, consider the following options:
      
      1. set up custom discovery (`find` directive with `include` or `exclude`)
      2. use a `src-layout`
      3. explicitly set `py_modules` or `packages` with a list of names
      
      To find more information, look for "package discovery" on setuptools docs.
      [end of output]
  
  note: This error originates from a subprocess, and is likely not a problem with pip.
error: subprocess-exited-with-error

× Getting requirements to build wheel did not run successfully.
│ exit code: 1
╰─> See above for output.

note: This error originates from a subprocess, and is likely not a problem with pip.

did you experience this?

@danielvegamyhre
Copy link
Contributor

@kannon92 I was able to resolve the issue in my previous comment by updating the pyproject.toml per this stackoverflow post: https://stackoverflow.com/questions/72294299/multiple-top-level-packages-discovered-in-a-flat-layout

Can we add this to the PR please?

@danielvegamyhre
Copy link
Contributor

Another issue: I have successfully installed the jobset package yet the create_jobset.py example script is unable to import it (I also tried importing it in a local python interpreter and it failed):

$ pip3 install .
...
Successfully built jobset
Installing collected packages: jobset
Successfully installed jobset-0.2.0
$ cd examples/
$ ls
create_jobset.py  README.md
$ python3 create_jobset.py
Traceback (most recent call last):
  File "<redacted>/go/src/sigs.k8s.io/jobset/sdk/python/examples/create_jobset.py", line 5, in <module>
    import jobset
ModuleNotFoundError: No module named 'jobset'

@danielvegamyhre
Copy link
Contributor

/assign

@kannon92
Copy link
Contributor Author

kannon92 commented Jul 6, 2023

Another issue: I have successfully installed the jobset package yet the create_jobset.py example script is unable to import it (I also tried importing it in a local python interpreter and it failed):

$ pip3 install .
...
Successfully built jobset
Installing collected packages: jobset
Successfully installed jobset-0.2.0
$ cd examples/
$ ls
create_jobset.py  README.md
$ python3 create_jobset.py
Traceback (most recent call last):
  File "<redacted>/go/src/sigs.k8s.io/jobset/sdk/python/examples/create_jobset.py", line 5, in <module>
    import jobset
ModuleNotFoundError: No module named 'jobset'

I am unable to reproduce this actually.

What version of python are you using? And are you using a virtual environment or installing into system python?

@danielvegamyhre
Copy link
Contributor

Another issue: I have successfully installed the jobset package yet the create_jobset.py example script is unable to import it (I also tried importing it in a local python interpreter and it failed):

$ pip3 install .
...
Successfully built jobset
Installing collected packages: jobset
Successfully installed jobset-0.2.0
$ cd examples/
$ ls
create_jobset.py  README.md
$ python3 create_jobset.py
Traceback (most recent call last):
  File "<redacted>/go/src/sigs.k8s.io/jobset/sdk/python/examples/create_jobset.py", line 5, in <module>
    import jobset
ModuleNotFoundError: No module named 'jobset'

I am unable to reproduce this actually.

What version of python are you using? And are you using a virtual environment or installing into system python?

I am using a virtualenv with Python 3.11.2

@danielvegamyhre
Copy link
Contributor

danielvegamyhre commented Jul 6, 2023

Interesting, I can import jobset from the sdk/python folder, but not from the sdk/python/examples folder.

I copied the create_jobset.py file from the examples folder to sdk/python and the import succeeds, but then the following error occurs on line 73 of the script:

...
list_namespaced_custom_object_with_http_info
    raise ApiTypeError(
kubernetes.client.exceptions.ApiTypeError: Got an unexpected keyword argument 'name' to method list_namespaced_custom_object

@danielvegamyhre
Copy link
Contributor

I think i found a small bug in the script, I will submit a quick PR

@danielvegamyhre
Copy link
Contributor

See #225 with the fix

@danielvegamyhre
Copy link
Contributor

I notice in my venv site-packages, for every package besides jobset there is a pattern of 2 directories existing: <package_name> and <package_name>-<version>.dist_info. However, for jobset, there is only jobset-0.2.0.dist_info, but no jobset:


(venv) $ ~/go/src/sigs.k8s.io/jobset/sdk/python$ ls venv/lib/python3.11/site-packages/
cachetools                          iniconfig                    pyasn1_modules                   requests_oauthlib
cachetools-5.3.1.dist-info          iniconfig-2.0.0.dist-info    pyasn1_modules-0.3.0.dist-info   requests_oauthlib-1.3.1.dist-info
certifi                             jobset-0.2.0.dist-info       __pycache__                      rsa
certifi-2023.5.7.dist-info          kubernetes                   py.py                            rsa-4.9.dist-info
charset_normalizer                  kubernetes-26.1.0.dist-info  _pytest                          setuptools
charset_normalizer-3.1.0.dist-info  oauthlib                     pytest                           setuptools-66.1.1.dist-info
coverage                            oauthlib-3.2.2.dist-info     pytest-7.4.0.dist-info           six-1.16.0.dist-info
coverage-7.2.7.dist-info            packaging                    pytest_cov                       six.py
dateutil                            packaging-23.1.dist-info     pytest_cov-4.1.0.dist-info       urllib3
_distutils_hack                     pip                          pytest-cov.pth                   urllib3-1.26.16.dist-info
distutils-precedence.pth            pip-23.0.1.dist-info         pytest_randomly-1.2.3.dist-info  websocket
google                              pkg_resources                pytest_randomly.py               websocket_client-1.6.1.dist-info
google_auth-2.21.0.dist-info        pluggy                       python_dateutil-2.8.2.dist-info  _yaml
google_auth-2.21.0-py3.9-nspkg.pth  pluggy-1.2.0.dist-info       PyYAML-6.0.dist-info             yaml
idna                                pyasn1                       requests
idna-3.4.dist-info                  pyasn1-0.5.0.dist-info       requests-2.31.0.dist-info

@kannon92
Copy link
Contributor Author

kannon92 commented Jul 6, 2023

Can you try again? I found a bug where I wasn't installing all the modules.

@danielvegamyhre
Copy link
Contributor

@kannon92 Nice the import issue seems to be resolved

@danielvegamyhre
Copy link
Contributor

/label tide/merge-method-squash
/lgtm
/approve

thanks for doing this! i'll patch this in to the last release as v0.2.1 and include #224 as well

@k8s-ci-robot k8s-ci-robot added tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. lgtm "Looks good to me", indicates that a PR is ready to be merged. labels Jul 6, 2023
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: danielvegamyhre, kannon92

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 6, 2023
@k8s-ci-robot k8s-ci-robot merged commit a5f3ee3 into kubernetes-sigs:main Jul 6, 2023
k8s-ci-robot added a commit that referenced this pull request Jul 6, 2023
…pstream-release-0.2

Automated cherry pick of #223: use pyproject.toml
@danielvegamyhre danielvegamyhre mentioned this pull request Dec 12, 2023
20 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants