Enable PyPI package management #1154
Conversation
| args = parser.parse_args() | ||
|
|
||
| # Move to the directory of this file. | ||
| os.chdir(os.path.dirname(os.path.abspath(__file__))) |
There was a problem hiding this comment.
Can these two directory-navigating steps be combined?
There was a problem hiding this comment.
Even better, don't do this at all, but pass the right directory (the one containing cwd) as the 'cwd' argument to subprocess.call.
There was a problem hiding this comment.
IMO, that breaks from the pattern of having executables under the tools folder be agnostic to where we are and having them always Do The Right Thing™.
There was a problem hiding this comment.
I don't understand what you mean, sorry. Using the cwd argument to subprocess.Popen doesn't mean you have to run this tool in a specific directory, it just means you don't change the current process's working directory for no good reason. Instead of doing 'os.chdir(pkgdir)' you pass 'cwd=pkgdir' to the two subprocess.call() calls.
|
@Yhg1s, would you mind taking a look at this? |
| os.chdir('../../../src/python/src') | ||
|
|
||
| # Make the push. | ||
| cmd = 'python setup.py sdist upload -r %s' % args.repository |
There was a problem hiding this comment.
Yeah, don't use 'sdist upload'. It's insecure. Use twine: https://pypi.python.org/pypi/twine
soltanmm
force-pushed
the
pypi
branch
3 times, most recently
from
fe9b407
to
f476e77
Compare
|
n.b. CI is failing, possibly due to #1037 |
|
I have hit the "Restart Job" button on https://travis-ci.org/grpc/grpc/jobs/56619466. :-) Where's the momentum on this at the moment? Are you still working through @Yhg1s' last round of comments or are you waiting on his next round? |
|
@nathanielmanistaatgoogle Waiting on the next round. |
|
My aversion to duplicated source resources in source control is strong. Do not copy the LICENSE file in source control if a simple file-copy-during-package-build is a perfectly reasonable alternative. |
| ------------ | ||
|
|
||
| Ensure that you've built the gRPC core from the root of the | ||
| `gRPC git repo`_ |
There was a problem hiding this comment.
Can you update this so that this says to either install the released deb packages on linux or build from source on other platforms; cf these instructiions
soltanmm
force-pushed
the
pypi
branch
5 times, most recently
from
33c8284
to
a92c93c
Compare
|
@tbetbetbe Is this kind of what you wanted? More detail? Less detail? More cross-references? Wrong cross-referenced resources? |
| ./configure | ||
| make && make install | ||
|
|
||
| Or install from a debian package: |
There was a problem hiding this comment.
Change the order, e.g.
- on linux, you can install using the deb package
... - otherwise, you can install from source
...
soltanmm
force-pushed
the
pypi
branch
2 times, most recently
from
a792105
to
bdf46d0
Compare
|
|
||
| Ensure that you have installed GRPC core. | ||
|
|
||
| Or install from a debian package: |
There was a problem hiding this comment.
'On debian linux systems, install from our released deb package'
soltanmm
force-pushed
the
pypi
branch
2 times, most recently
from
10611ef
to
b5298ba
Compare
| # Move to the root directory of Python GRPC. | ||
| pkgdir = os.path.join(os.path.dirname(os.path.abspath(__file__)), | ||
| '../../../src/python/src') | ||
| os.chdir(pkgdir) |
There was a problem hiding this comment.
I'm sure I left a comment about this, but I can't find it now (nor the whole conversation it happened in) in GitHub's awful interface.
There is no need to do this os.chdir. Simply pass cwd=pkgdir to the two subprocess.call() calls below (and pass os.path.join(pkgdir, 'dist') to shutil.rmtree on line 39.)
Enable PyPI package management
|
Thank you for your pull request. Before we can look at your contribution, we need to ensure all contributors are covered by a Contributor License Agreement. After the following items are addressed, please respond with a new comment here, and the automated system will re-verify.
Regards, |
@nathanielmanistaatgoogle Please review, inevitably have actionable feedback, deal with the back and forth, and then merge at some point in the future! :-D