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

Fix few things and python3 compatibility #7

Merged
merged 7 commits into from
Feb 2, 2018
Merged

Fix few things and python3 compatibility #7

merged 7 commits into from
Feb 2, 2018

Conversation

BLaurent
Copy link

Hi,

I found some issues, Please feel free to comment.

Best regards
B

@j12y
Copy link
Owner

j12y commented Feb 2, 2018

Apparently I wasn't watching this repos. Thanks @BLaurent. The internal github repository is unfortunately disconnected from the public one so not everything is published here yet. I had done some verification on Python 3.6 but I'll take a closer look at what you have here to see if you caught anything else since the integration tests are still pretty limited.

Thanks!

Copy link
Owner

@j12y j12y left a comment

Choose a reason for hiding this comment

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

Thanks again Benoit, I'll merge these in. Always good to get more eyes on it and these changes make sense.

@@ -1,6 +1,7 @@

import os
import urlparse

from future.moves.urllib.parse import urlparse
Copy link
Owner

Choose a reason for hiding this comment

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

Yep, using this one now. I considered whether to use six but seemed to make no difference.

@@ -83,7 +84,7 @@ def get_query_zone_id(self):

def get_ingest_uri(self):
"""
Return the uri used for ingesting data into time series
Return the uri used for ingesting data into time series
Copy link
Owner

Choose a reason for hiding this comment

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

I guess I haven't run the linter in awhile.

@j12y j12y merged commit 022b84b into j12y:master Feb 2, 2018
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