Skip to content

adding python version environmental markers in the new style#16235

Merged
mehrdada merged 1 commit intogrpc:masterfrom
a-musing-moose:master
Aug 3, 2018
Merged

adding python version environmental markers in the new style#16235
mehrdada merged 1 commit intogrpc:masterfrom
a-musing-moose:master

Conversation

@a-musing-moose
Copy link
Copy Markdown
Contributor

A follow up on #16133 which I have tested locally to ensure that when doing pip install pulls futures on on Python versions less that 3.2 (when it was added to the stdlib) and enum34 when Python < 3.4.

@mehrdada - i'm hoping this one will pass the distrib tests as I was unable to work out how to run them locally but a pip install does do what I would expect.

Note: It does require a recent version of setuptools. So there may still be drama but I am hoping not.

Copy link
Copy Markdown
Contributor

@mehrdada mehrdada left a comment

Choose a reason for hiding this comment

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

LGTM — I'll merge and trigger distribtests to see.

@mehrdada
Copy link
Copy Markdown
Contributor

mehrdada commented Aug 3, 2018

@mehrdada
Copy link
Copy Markdown
Contributor

mehrdada commented Aug 3, 2018

@mehrdada mehrdada added the release notes: yes Indicates if PR needs to be in release notes label Aug 3, 2018
@mehrdada
Copy link
Copy Markdown
Contributor

mehrdada commented Aug 3, 2018

Distribtests succeeded. Will merge as soon as the PR required tests pass.

Thanks for the PR!

@grpc-testing
Copy link
Copy Markdown

Objective-C binary sizes
*****************STATIC******************
  New size                      Old size
 1,980,683      Total (=)      1,980,683

 No significant differences in binary sizes

***************FRAMEWORKS****************
  New size                      Old size
10,811,774      Total (>)     10,811,764

 No significant differences in binary sizes


@mehrdada mehrdada merged commit 7c3d13d into grpc:master Aug 3, 2018
@mehrdada
Copy link
Copy Markdown
Contributor

mehrdada commented Aug 3, 2018

Packaging change: relevant tests (artifact/distribtests) are separately run and passed in the above links.

@a-musing-moose
Copy link
Copy Markdown
Contributor Author

That's great news @mehrdada. I'm not sure of the process from here on. Do you have a schedule for release?

@mehrdada
Copy link
Copy Markdown
Contributor

mehrdada commented Aug 3, 2018

Not much. It'll automatically get released as part of the next release branch (and in tonight's nightly build from master on https://packages.grpc.io). We have a 6 week release cycle and shipped 1.14 right now. The 1.15-pre1 is planned for beginning of September and the final 1.15.0 is expected mid September. Do you have a more urgent need than that?

@mehrdada
Copy link
Copy Markdown
Contributor

mehrdada commented Aug 3, 2018

/cc @hsaliak for publishing the release schedule.

@a-musing-moose
Copy link
Copy Markdown
Contributor Author

Yes and no. It can certainly live with a git based install or even a vendored pre-built wheel until the official release. Thanks for your help.

@mehrdada
Copy link
Copy Markdown
Contributor

mehrdada commented Aug 4, 2018

The nightlies are out of the oven with this PR. You can try pip installing by adding an pip index URL as instructed on this page: https://packages.grpc.io/archive/2018/08/7c3d13d440debe4b67e05e186a8e8e0d2b4f4918-c51a137a-3531-4f87-9670-7c4f36009649/index.xml

@mehrdada
Copy link
Copy Markdown
Contributor

mehrdada commented Aug 9, 2018

@a-musing-moose Had to revert this because pip install on macOS on Python3.4 failed to run on our CI systems, which makes me feel the downside is much higher than upside. Might I ask what exactly we gain from this and is there another way to achieve compatibility with the specific tool you mentioned before?

@a-musing-moose
Copy link
Copy Markdown
Contributor Author

Hi @mehrdada Well that is a shame.

The issue I am trying to correct is that I am unable to install this package with poetry. Details of the issue can be found here: python-poetry/poetry#328

This leave me with an awkward work around whereby I use Poetry to install all the project dependencies except grpcio and grpcio-tools packages.

@mehrdada
Copy link
Copy Markdown
Contributor

mehrdada commented Aug 9, 2018

@a-musing-moose We might be able to work around it by having both an imperative logic that ensures those tuples are eliminated if PY3, and keep your changes too... Does that satisfy poetry?

Something like:

INSTALL_REQUIRES = (
    "six>=1.5.2",
    "futures>=2.2.0 ; python_version<'3.2'",	
    "enum34>=1.0.4 ; python_version<'3.4'"	
)

if not PY3:
  # forces setuptools not to try installing `futures` on Python 3
  # even if it is too dumb to figure it out by itself.
  INSTALL_REQUIRES = ("six>=1.5.2",)

@a-musing-moose
Copy link
Copy Markdown
Contributor Author

@mehrdada to be honest I'm not entirely sure. I am unfamiliar with how poetry parses the setup.py to extract the list of dependencies. It may work but I cannot make any guarantees.

I would however by happy to test/trail something along those lines.

@mehrdada
Copy link
Copy Markdown
Contributor

mehrdada commented Aug 9, 2018

If you tried above and it worked on poetry, just send us another PR.

@lock lock bot locked as resolved and limited conversation to collaborators Nov 7, 2018
@lock lock bot unassigned mehrdada Nov 7, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants