You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2020/10/07 13:35:02 UTC

[GitHub] [pulsar] languitar opened a new pull request #8213: [Issue 7050][python] Limit enum34 installation via environment markers

languitar opened a new pull request #8213:
URL: https://github.com/apache/pulsar/pull/8213


   Fixes #7050 
   
   ### Motivation
   
   To prevent installing enum34 on systems where it is not needed, declare the dependency with environment markers. Installing certain versions of enum34 on systems where it is not needed at all, might result in compile time issues. This is the recommended approach discussed here:
   https://github.com/python-poetry/poetry/issues/1122
   
   ### Modifications
   
   * `setup.py`: use environment markers for enum34 instead of custom logic that is not preserved in pypi metadata for the wheel
   
   ### Verifying this change
   
   - [ ] Make sure that the change passes the CI checks.
   
   This change is already covered by existing tests, such as all python tests
   
   ### Does this pull request potentially affect one of the following parts:
   
   *If `yes` was chosen, please highlight the changes*
   
     - Dependencies (does it add or upgrade a dependency): no
     - The public API: no
     - The schema: no
     - The default values of configurations: no
     - The wire protocol: no
     - The rest endpoints: no
     - The admin cli options: no
     - *Anything that affects deployment*: don't know
   
   ### Documentation
   
     - Does this pull request introduce a new feature? no
     - If yes, how is the feature documented? not applicable
     - If a feature is not applicable for documentation, explain why? Only packaging change
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] rosejn commented on pull request #8213: [Issue 7050][python] Limit enum34 installation via environment markers

Posted by GitBox <gi...@apache.org>.
rosejn commented on pull request #8213:
URL: https://github.com/apache/pulsar/pull/8213#issuecomment-707882188


   Hi guys, after my team just waisted way too much time battling with this issue I was frustrated and trying to encourage forward motion on this PR, so my comment wasn't very clear.  I was just hoping to encourage movement on this PR, and if there was a problem with this is I wanted to volunteer to do whatever was needed to help get a fix merged.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] tuteng commented on pull request #8213: [Issue 7050][python] Limit enum34 installation via environment markers

Posted by GitBox <gi...@apache.org>.
tuteng commented on pull request #8213:
URL: https://github.com/apache/pulsar/pull/8213#issuecomment-706511428


   /pulsarbot run-failure-checks


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] sijie commented on pull request #8213: [Issue 7050][python] Limit enum34 installation via environment markers

Posted by GitBox <gi...@apache.org>.
sijie commented on pull request #8213:
URL: https://github.com/apache/pulsar/pull/8213#issuecomment-707507679


   @rosejn If you have a better solution than this pull request, feel free to submit one. So the community can take a look at the change and merge it if it solves the problem.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] sijie merged pull request #8213: [Issue 7050][python] Limit enum34 installation via environment markers

Posted by GitBox <gi...@apache.org>.
sijie merged pull request #8213:
URL: https://github.com/apache/pulsar/pull/8213


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] languitar commented on pull request #8213: [Issue 7050][python] Limit enum34 installation via environment markers

Posted by GitBox <gi...@apache.org>.
languitar commented on pull request #8213:
URL: https://github.com/apache/pulsar/pull/8213#issuecomment-704958966


   Can't see how the failing test is related to this pull request


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] rosejn commented on pull request #8213: [Issue 7050][python] Limit enum34 installation via environment markers

Posted by GitBox <gi...@apache.org>.
rosejn commented on pull request #8213:
URL: https://github.com/apache/pulsar/pull/8213#issuecomment-707365065


   This enum34 dependency is causing major issues for our projects, and this environment marker would fix that.  I see people have been having issues with enum34 for over a year now.  Can we not just get rid of it, or at minimum only use it as a dependency when using an old python version?  I'm happy to help get this moving in any way possible, as we have already lost many man hours dealing with the fallout.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] rosejn edited a comment on pull request #8213: [Issue 7050][python] Limit enum34 installation via environment markers

Posted by GitBox <gi...@apache.org>.
rosejn edited a comment on pull request #8213:
URL: https://github.com/apache/pulsar/pull/8213#issuecomment-707882188


   Hi guys, after my team just waisted way too much time battling with this issue I was frustrated and trying to encourage forward motion on this PR, so my comment wasn't very clear.  If there was something holding this back I wanted to volunteer to do whatever was needed to help get a fix merged.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] languitar commented on pull request #8213: [Issue 7050][python] Limit enum34 installation via environment markers

Posted by GitBox <gi...@apache.org>.
languitar commented on pull request #8213:
URL: https://github.com/apache/pulsar/pull/8213#issuecomment-707531031


   > @rosejn If you have a better solution than this pull request, feel free to submit one. So the community can take a look at the change and merge it if it solves the problem.
   
   Is there anything wrong with this PR that would require a different solution? According to @rosejn the marker that I introduced would solve the problem.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] sijie commented on pull request #8213: [Issue 7050][python] Limit enum34 installation via environment markers

Posted by GitBox <gi...@apache.org>.
sijie commented on pull request #8213:
URL: https://github.com/apache/pulsar/pull/8213#issuecomment-707863709


   @languitar I don't think so. I wasn't sure what does his comment mean. So I was asking if he is thinking about a different solution than this PR.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] rosejn commented on pull request #8213: [Issue 7050][python] Limit enum34 installation via environment markers

Posted by GitBox <gi...@apache.org>.
rosejn commented on pull request #8213:
URL: https://github.com/apache/pulsar/pull/8213#issuecomment-708046167


   Thanks guys!  I'm happy to test this out to double check that things work as hoped for.  I've just pulled the latest on master and run the docker-build.sh script in pulsar-client-cpp, where it successfully built.  Anyone know how to then try this out as a poetry dependency?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org