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

Including sentAt in the Batch body of the HTTP request forces the use of a segment server computed timestamp #340

Closed
sromocki opened this issue Apr 20, 2022 · 4 comments · Fixed by #345

Comments

@sromocki
Copy link

Is it possible to not include the sentAt() in the Batch body as this then forces all requests to use Segment's calculated timestamp versus the client provided timestamp if they try and set one. Usually when a client sets a timestamp its for historical reasons and so the client provided timestamp should be the single source of truth without any calculations around clock skew.

@sromocki
Copy link
Author

Instead of removing it completely you could allow users of the library to set the sentAt timestamp so that we can set it to null if we're doing a historical import.

@pooyaj
Copy link
Contributor

pooyaj commented Apr 29, 2022

hey @sromocki, could you please check this PR to see if it solves for your case: #345

@sromocki
Copy link
Author

sromocki commented May 2, 2022

@pooyaj It needs to be accessible from the client library. I need to be able to do builder.sentAt(null).

@segmentio segmentio deleted a comment from sromocki May 4, 2022
@nd4p90x nd4p90x linked a pull request May 4, 2022 that will close this issue
@wenxi-zeng
Copy link

@sromocki just a clarification on the changes:

  • you should now be able to modify the sentAt value in the batch payload through builder.sentAt(value) by passing a non-null value
  • however, by default or by passing a null value will force the value to be new Date()

if you don't have an sentAt value and want to prevent a Segment computed value, try pass an empty string.

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 a pull request may close this issue.

3 participants