Skip to content
This repository was archived by the owner on Sep 17, 2025. It is now read-only.

Fix build errors#362

Merged
reyang merged 2 commits intomasterfrom
py37
Oct 26, 2018
Merged

Fix build errors#362
reyang merged 2 commits intomasterfrom
py37

Conversation

@reyang
Copy link
Copy Markdown
Contributor

@reyang reyang commented Oct 25, 2018

I'm getting build errors which seems to be related to the recent py3.6 upgrade in CI:
opencensus/stats/exporters/stackdriver_exporter.py:23:42: W606 'async' and 'await' are reserved keywords starting with Python 3.7
opencensus/stats/exporters/stackdriver_exporter.py:112:28: W606 'async' and 'await' are reserved keywords starting with Python 3.7
opencensus/stats/exporters/stackdriver_exporter.py:450:5: F841 local variable 'exception' is assigned to but never used
opencensus/trace/propagation/google_cloud_format.py:22:17: W605 invalid escape sequence '/'
opencensus/trace/propagation/google_cloud_format.py:22:40: W605 invalid escape sequence '\d'

async is becoming a keyword in python3.7, this blocks the current CI (which is using the latest py3.6, and failed due to warnings).

@reyang
Copy link
Copy Markdown
Contributor Author

reyang commented Oct 25, 2018

@liyanhui1228 do we want to revisit the names? I'm open for discussion on what's the best naming.

@reyang reyang closed this Oct 25, 2018
@reyang reyang reopened this Oct 25, 2018

_TRACE_CONTEXT_HEADER_NAME = 'X-Cloud-Trace-Context'
_TRACE_CONTEXT_HEADER_FORMAT = '([0-9a-f]{32})(\/([0-9a-f]{16}))?(;o=(\d+))?'
_TRACE_CONTEXT_HEADER_FORMAT = r'([0-9a-f]{32})(\/([0-9a-f]{16}))?(;o=(\d+))?'
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is a separate issue which was caught by the latest pylint.
The string literal used for regular expression was not properly escaped.

@reyang reyang changed the title Avoid using keyword in package names Fix build errors Oct 25, 2018
@songy23 songy23 requested a review from c24t October 25, 2018 22:17
Copy link
Copy Markdown
Member

@c24t c24t left a comment

Choose a reason for hiding this comment

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

What do you think about moving AsyncTransport and SyncTransport into the transport module and losing sync and async altogether? In any case the rename LGTM.

@reyang reyang mentioned this pull request Oct 26, 2018
@reyang reyang merged commit 1df8f58 into master Oct 26, 2018
@reyang reyang deleted the py37 branch October 26, 2018 16:39
@reyang reyang mentioned this pull request Oct 26, 2018
c24t pushed a commit to c24t/opencensus-python that referenced this pull request Nov 21, 2018
* avoid using keyword in package names

* fix build error
c24t pushed a commit to c24t/opencensus-python that referenced this pull request Nov 21, 2018
* avoid using keyword in package names

* fix build error
c24t pushed a commit that referenced this pull request Nov 28, 2018
* avoid using keyword in package names

* fix build error
@c24t c24t mentioned this pull request Dec 11, 2018
@zoidyzoidzoid
Copy link
Copy Markdown

I'm not a fan of async_.py, but we can fix it in another PR.

@reyang
Copy link
Copy Markdown
Contributor Author

reyang commented May 2, 2019

I'm not a fan of async_.py, but we can fix it in another PR.

Me too! Let's see if we can get rid of it in #642.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants