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

Sync with upstream and support Python 3 #1

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Sync with upstream and support Python 3 #1

wants to merge 2 commits into from

Conversation

streeter
Copy link

@streeter streeter commented May 3, 2016

The upstream protopc repo moved to https://github.com/google/protorpc, and has updates to support python 3.

This brings the repository up-to-date, and adds a .travis.yml to run the tests (which passed locally for me).

@jeremydw
Copy link
Owner

jeremydw commented May 4, 2016

Thanks for sending this over -- I forked this because at the time I was unable to run protorpc out of an App Engine environment. I just tried cloning https://github.com/google/protorpc and ensuring six was updated to the latest and looks like that problem's since been fixed as I can from protorpc import messages and use the messages.Message class as usual.

Wonder if we can just upgrade Grow's requirements.txt to point upstream to google/protorpc and delete this GitHub repository? (We can leave protorpc-standalone on PyPi for any build tools that depend on it but no one should be depending on this GH mirror directly.)

@streeter
Copy link
Author

streeter commented May 4, 2016

I'm not certain, but I think they added support for not having the google.net.proto package installed [https://github.com/google/protorpc/blob/master/protorpc/google_imports.py](which I assume is inside of GAE).

So If you're not using ProtocolBuffer anywhere, you should probably be ok.

@jeremydw
Copy link
Owner

jeremydw commented May 6, 2016

Okay, thanks for looking at that. I gave this a go from the latest release
on PyPi and it looks like there is still an ImportError because the
ImportError is masked but code still depends on ProtocolBuffer (
https://github.com/google/protorpc/blob/981174b77c87c222ac38fdc156bd7b2607b31531/protorpc/protobuf.py#L55
).

I should be able to work around this, will update this issue.

On Wed, May 4, 2016 at 4:34 AM, Chris Streeter notifications@github.com
wrote:

I'm not certain, but I think they added support for not having the
google.net.proto package installed
https://github.com/google/protorpc/blob/master/protorpc/google_imports.py
http://which%20I%20assume%20is%20inside%20of%20GAE.

So If you're not using ProtocolBuffer anywhere, you should probably be ok.


You are receiving this because you commented.
Reply to this email directly or view it on GitHub
#1 (comment)

jeremydw

jeremydw added a commit to grow/grow that referenced this pull request May 6, 2016
@jeremydw
Copy link
Owner

jeremydw commented May 6, 2016

I went ahead and fixed the issue. Looks like protorpc.wsgi.service was what contained the App Engine dependency and we only depended on that in dead code, which I removed a while back. grow/grow@138e4c0 removes the dependency on our forked protorpc, so we should be able to go ahead and delete the entire protorpc-standalone repo now.

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