-
Notifications
You must be signed in to change notification settings - Fork 6.4k
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
Fixup nox #462
Conversation
Change-Id: I6bf9a8acb9ba7d067b3095b5857094cbc322ff58
Change-Id: Ie0df6747050035b2ef5f937951d5ff955073e6d4
Change-Id: I0d3bdf3d6842339d04abc4ee6ddb26b8f44be3e5
@@ -70,7 +70,7 @@ def main(project_id, instance_id, table_id): | |||
row = table.row(row_key) | |||
row.set_cell( | |||
column_family_id, | |||
column_id.encode('utf-8'), | |||
column_id, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this have some six
magic for python2 compatibility?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think Jon is just fixing a double encode? I think in general if you want Python strings as bytes, do the encode, no six necessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah cool. I totally didn't look at the context ^_^;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah fixing a double encode. No idea how this worked on py2.7 other than luck.
hooray. code lgtm. running it locally if you want to wait for that but I'm guessing any problems there will be doc issues anyway. |
|
||
|
||
def pytest_ignore_collect(path, config): | ||
"""Skip App Engine tests in python 3 and if no SDK is available.""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"and" -> "or"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
LGTM if LGTTravis |
Change-Id: I02a53961b6411247ef06d84dad7b533cb97d89f7
@dpebot merge when travis passes |
Okay! I'll merge when all statuses are green. |
Co-authored-by: AJ Morozoff <amorozoff@google.com> Co-authored-by: Anthonios Partheniou <partheniou@google.com>
Source-Link: googleapis/synthtool@4760d8d Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:f0e4b51deef56bed74d3e2359c583fc104a8d6367da3984fc5c66938db738828 Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
Source-Link: googleapis/synthtool@4760d8d Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:f0e4b51deef56bed74d3e2359c583fc104a8d6367da3984fc5c66938db738828 Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com> Co-authored-by: Anthonios Partheniou <partheniou@google.com>
Co-authored-by: AJ Morozoff <amorozoff@google.com> Co-authored-by: Anthonios Partheniou <partheniou@google.com>
Source-Link: googleapis/synthtool@4760d8d Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:f0e4b51deef56bed74d3e2359c583fc104a8d6367da3984fc5c66938db738828 Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
Co-authored-by: AJ Morozoff <amorozoff@google.com> Co-authored-by: Anthonios Partheniou <partheniou@google.com>
This enables grpc tests on python 3 and unifies the app engine and python 2.7 test suite (woohoo).