Skip to content

publish wheels #1128

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

Closed
wants to merge 1 commit into from
Closed

publish wheels #1128

wants to merge 1 commit into from

Conversation

dimbleby
Copy link

publish wheels

rather than make every consumer build the wheel themselves, publish it once and for all

@robsdedude
Copy link
Member

robsdedude commented Dec 3, 2024

Thanks for taking the time to craft a PR.

As you can see on PyPI the wheel for neo4j are already published. For the legacy package neo4j-driver I didn't go for uploading wheels for these reasons:

  • Support for neo4j-driver will be dropped at the end of version 5. neo4j is a package alias and can be swapped in without code changes. So we expect users to migrate to the new package name. This brings me to point 2:
  • Less comforts can increase migration velocity.
  • Finally, the installation of the legacy package emits a deprecation warning. This isn't possible with pre-build wheels. Granted, pip doesn't show the warning. However for users running their env setup (for instance in CI) with -W error, the installation will fail and let them know that they should change the name of the package.

@dimbleby
Copy link
Author

dimbleby commented Dec 3, 2024

I don't find any of that convincing to be honest, but I don't care to argue about it - this isn't very important.

If you want a deprecation warning that people will actually see, perhaps putting it in __init__.py so that it is shown on package import would make sense.

@robsdedude
Copy link
Member

robsdedude commented Dec 3, 2024

If you want a deprecation warning that people will actually see, perhaps putting it in init.py so that it is shown on package import would make sense.

That's in place as well 😅

if _deprecated_package:
_deprecation_warn(
"The neo4j driver was installed under the package name `noe4j-driver` "
"which is deprecated and will stop receiving updates starting with "
"version 6.0.0. Please install `neo4j` instead (which is an alias, "
"i.e., a drop-in replacement). See https://pypi.org/project/neo4j/ ."
)

I don't find any of that convincing to be honest, but I don't care to argue about it - this isn't very important.

I'm still curious to hear your view. Do you have or see a use-case where switching over to neo4j is not an option?

@dimbleby
Copy link
Author

dimbleby commented Dec 3, 2024

I am not a user of neo4j and have no views on switching to or from one package or the other.

I just spotted that this package is published without a wheel, that is usually undesirable, and so I propose to fix it.

I am sceptical that the minor barrier of having users build their own wheel is doing anything at all to help your deprecation, I would guess it it is more likely just making life ever so slightly worse for the people who are not yet ready to move

But as I say this is not important - "minor" and "ever so slightly worse" by my own reasoning! If you are deliberately choosing not to publish a wheel and think that is right for you - then I am happy to respectfully disagree and leave it there.

@robsdedude
Copy link
Member

Thanks for you for taking the time to share your view. After thinking about it for longer, I came to the conclusion that you are right.

Before I can review the PR, however, I have to ask you to sign our Contributor License Agreement (CLA): https://neo4j.com/developer/cla/

@dimbleby
Copy link
Author

dimbleby commented Dec 4, 2024

I will not be signing the contributor license agreement. You are welcome to this change if you want it.

@robsdedude
Copy link
Member

Thanks 😅 I was almost expecting such reply. I will close this PR and take care of publishing the wheels myself. It's not a big change anyway.

Again: thank you very much for taking the time to open the PR and to discuss the matter with me. This is much appreciated 🙇

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.

2 participants