Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings
This repository was archived by the owner on Oct 29, 2024. It is now read-only.

Fix unpacking of negative timestamps from msgpack#846

Open
bitoffdev wants to merge1 commit intoinfluxdata:master
base:master
Choose a base branch
Loading
frombitoffdev:fix-msgpack-timestamp-parse

Conversation

bitoffdev
Copy link

@bitoffdevbitoffdev commentedAug 23, 2020
edited
Loading

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. After rebasing onto master, coverage is also passing.
  • 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.

frominfluxdbimportInfluxDBClientjson_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;')assertnext(result['cpu_load_short'])== {'time':'1960-11-10T23:00:12.123450Z','value':0.64}print('Negative timestamps were handled correctly!')

lovasoa reacted with thumbs up emoji
@bitoffdevbitoffdevforce-pushed thefix-msgpack-timestamp-parse branch fromedb02be tofa1e7e7CompareDecember 2, 2020 03:30
@bitoffdev
Copy link
Author

I rebased this PR onto master, which fixed the failing code coverage test.@sebito91, would you be able to take a look at this? My apologies for the at-mention, but I haven't been able to get any eyes on this patch. Let me know if there is a better way to request reviews for this project. Thanks!

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

@aviauaviauAwaiting requested review from aviauaviau is a code owner

@sebito91sebito91Awaiting requested review from sebito91

@xginn8xginn8Awaiting requested review from xginn8xginn8 is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

1 participant
@bitoffdev

[8]ページ先頭

©2009-2025 Movatter.jp