-
Notifications
You must be signed in to change notification settings - Fork 524
Conversation
as introduced in influxdata/influxdb#2696
Working again with influxdata/influxdb@246ce61 Signed-off-by: Petr Štetiar <ynezz@true.cz>
…e-protocol Conflicts: tests/influxdb/client_test.py
* ``if value`` can lead to errors
* Added epoch parameter to ``query`` * Removed test_write_points_with_precision as queries now always return nanosecond precision See doc: > The format of the returned timestamps complies with RFC3339, and > has nanosecond precision.
@@ -96,7 +97,8 @@ def __init__(self, | |||
|
|||
self._headers = { | |||
'Content-type': 'application/json', | |||
'Accept': 'text/plain'} | |||
'Accept': 'text/plain' |
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.
Any specific reason for this new line?
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.
Not really, this was included in a Formating improvements
commit.
Try to return a text aka unicode object from the given data. | ||
""" | ||
if isinstance(data, binary_type): | ||
return data.decode('utf-8', 'replace') |
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.
Maybe would it be worth noting in the documentation that the library does a replace when decoding utf-8
since that could be a surprise for some users. Thoughts?
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'd say make it optional. May be for some users it's going to be helpful if they know that the data they're feeding is not sucessfully processed.
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'll disable it for now. We can add an option later. I'm not sure this is a feature we want.
I'm probably going to have an in-depth look at this tommorow with some manual testing :). It looks really nice and clean at first reading. |
* Update delete_series query. Instead of an id, it has FROM <series> and WHERE <tag>=<value> clauses. Update tests. * Remove grant_admin_privileges method. Cluster administration privileges are granted upon user creation. Remove tests. * Update create_user method to accept an optional argument to determine whether the user should be granted cluster administration privileges. Update tests. * Fix the test_write_check_read test case in the tests against a real server. * Fix revoke_admin_privileges test. It was failing because of the grant_admin_privileges method malfunction.
I'm waiting for tests and I'll merge |
Thanks to @trehn and @ynezz
\cc @ChristopherRabotin @georgijd Any feedback? Would any of you guys want to test?