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 unpacking of negative timestamps from msgpack #846

Open
wants to merge 1 commit into
base: master
from

Conversation

@bitoffdev
Copy link

@bitoffdev bitoffdev commented Aug 23, 2020

In #734, support was added to this python client for messagepack, but it does not match the serialization from influxdb for negative timestamps, because it interprets the timestamps as unsigned.

Influxdb uses the tinylib/msgp library to serialize timestamps. Here is an extract of how the timestamps are serialized.

Time is encoded as Unix time, which means that location (time zone) data is removed from the object. The encoded object itself is 12 bytes: 8 bytes for a big-endian 64-bit integer denoting seconds elapsed since "zero" Unix time, followed by 4 bytes for a big-endian 32-bit signed integer denoting the nanosecond offset of the time. This encoding is intended to ease portability across languages. msgp/write.go

This patch updates the python client to correctly use signed second and nanosecond fields.


Contributor checklist
  • Builds are passing
    • The only failing test is coverage, which is already failing on master. Fixing the existing issue with coverage is out of scope for this pull request.
  • New tests have been added (for feature additions)
    • I modified an existing test to ensure that negative timestamps are parsed correctly when we get a msgpack response.

Extra Manual Verification

As a sanity check, I also tested the following script against influxdb (via Docker) before and after my changes, and the result was as expected: negative timestamps fail before this patch and work correctly following this patch.

from influxdb import InfluxDBClient
json_body = [
    {
        "measurement": "cpu_load_short",
        "tags": {
            "host": "server01",
            "region": "us-west"
        },
        "time": "1960-11-10T23:00:12.12345Z",
        "fields": {
            "value": 0.64
        }
    }
]
client = InfluxDBClient('localhost', 8086, 'root', 'root', 'example')
client.drop_database('example')
client.create_database('example')
client.write_points(json_body)
result = client.query('select value from cpu_load_short;')
assert next(result['cpu_load_short']) == {'time': '1960-11-10T23:00:12.123450Z', 'value': 0.64}
print('Negative timestamps were handled correctly!')
@bitoffdev bitoffdev requested review from aviau, sebito91 and xginn8 as code owners Aug 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

1 participant
You can’t perform that action at this time.