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

Fix multi python package publishing #201

Merged
merged 3 commits into from
Nov 23, 2021

Conversation

davidbrochart
Copy link
Contributor

We now build Python packages in separate distribution directories, and upload them to PyPI independently. This also means publishing assets to PyPI and to NPM separately.

Fixes #200

@codecov-commenter
Copy link

codecov-commenter commented Nov 5, 2021

Codecov Report

Merging #201 (300724f) into master (25f51b4) will decrease coverage by 0.03%.
The diff coverage is 87.50%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #201      +/-   ##
==========================================
- Coverage   84.85%   84.81%   -0.04%     
==========================================
  Files          19       19              
  Lines        2271     2272       +1     
  Branches      280      281       +1     
==========================================
  Hits         1927     1927              
  Misses        248      248              
- Partials       96       97       +1     
Impacted Files Coverage Δ
jupyter_releaser/lib.py 83.96% <83.33%> (-0.23%) ⬇️
jupyter_releaser/cli.py 91.30% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 25f51b4...300724f. Read the comment docs.

@davidbrochart davidbrochart force-pushed the fix_multipy branch 5 times, most recently from 9d7f567 to 5d53e01 Compare November 5, 2021 18:16
@davidbrochart
Copy link
Contributor Author

davidbrochart commented Nov 5, 2021

The problem is that no matter if we build packages in separate directories, we install them in a common directory before uploading them to PyPI, so it's useless.
What I did is to provide a list of packages in the form "package_path:package_name", like so:
https://github.com/jupyter-server/jupyverse/blob/d02cde63d0bf2e47fb5d9310b74a9fc6c95b9ce1/pyproject.toml#L22
I added the package_name because we need the package name to map the distribution files to their PyPI token.

I checked that it works fine with jupyverse.

Comment on lines +423 to +425
if name.startswith(
python_package_name
): # FIXME: not enough to know it's the right package
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The distribution file name is something like my_package-0.1.2.tar.gz or my_package-0.1.2-py3-none-any.whl, and we could imagine a package name being my_package and another one being my_package_2, and both would match this condition, so this is not great. Maybe we could include the version spec in the condition, but at first sight tests were failing when doing so.

@davidbrochart davidbrochart marked this pull request as ready for review November 12, 2021 10:57
@jtpio
Copy link
Member

jtpio commented Nov 15, 2021

Thanks @davidbrochart for looking into this.

@@ -4,7 +4,7 @@ runs:
using: "composite"
steps:
- name: install-releaser
uses: jupyter-server/jupyter_releaser/.github/actions/install-releaser@v1
uses: davidbrochart/jupyter_releaser/.github/actions/install-releaser@fix_multipy
Copy link
Member

Choose a reason for hiding this comment

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

We'll need to revert that one (and similar below) to point to the releaser actions.

@davidbrochart
Copy link
Contributor Author

This is ready to be merged, and jupyverse depends on it. Given that the multi-python packages feature is only used by jupyverse AFAIK, I think the current limitation is acceptable, and we can improve it in the future. The limitation is that the package names, given in the form:

python_packages = [".:jupyverse", "plugins/auth:fps-auth", "plugins/contents:fps-contents"]

must not start with the same name, e.g. something like that wouldn't work:

python_packages = [".:jupyverse", "plugins/auth:fps-auth", "plugins/contents:fps-auth2"]

@blink1073 blink1073 added the enhancement New feature or request label Nov 23, 2021
@blink1073 blink1073 added this to the 0.10 milestone Nov 23, 2021
Copy link
Contributor

@blink1073 blink1073 left a comment

Choose a reason for hiding this comment

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

Thanks!

@blink1073 blink1073 added bug Something isn't working and removed enhancement New feature or request labels Nov 23, 2021
@blink1073 blink1073 modified the milestones: 0.10, 0.9 Nov 23, 2021
@blink1073 blink1073 merged commit 1bc9790 into jupyter-server:master Nov 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Packages uploaded multiple times
4 participants