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

Support relative end_date syntax #27

Open
pnadolny13 opened this issue Mar 3, 2023 · 2 comments
Open

Support relative end_date syntax #27

pnadolny13 opened this issue Mar 3, 2023 · 2 comments

Comments

@pnadolny13
Copy link
Collaborator

Initially discussed in #26 (comment) and meltano/sdk#922.

TLDR;

What do you think of us defining end_date to be either an ISO 8601 date or datetime value (as is expected for start_date) or an ISO 8601 time interval. We could then use the following interpretation logic:
......
So, from the above, our proposed default of "5 minutes ago" would be "end_date": "-PT5M". To include exactly 10 days of data from the provided start_date, the end date could use a interval:"end_date": "P10D".

@pnadolny13 pnadolny13 changed the title Improved end_date syntax Support relative end_date syntax Mar 3, 2023
@pnadolny13
Copy link
Collaborator Author

@aaronsteers I was thinking of making the buffer from #26 configurable in a future iteration by adding another config key for something like buffer_s but your proposed solution sounds like it would achieve it without having a new key and it would also be a good implementation reference for meltano/sdk#922.

One thing I'm considering is that as the tap developer I think I still want to force a buffer like this to avoid users shooting themselves in the foot. It took a fair amount of iteration and monitoring for me to figure out this was the problem, and I still need to see it work in prod to know for sure, so I'd want to make that an implementation detail of this tap. If users provide an end_date newer than 5 mins then I'd still automatically force the 5 mins buffer for safety. If we find cloudwatch docs explaining it with a more accurate SLA (likely in the seconds range vs mins) then I can shorten the hard coded buffer but I wouldnt want to give the user unsafe configuration options.

@aaronsteers
Copy link

One thing I'm considering is that as the tap developer I think I still want to force a buffer like this to avoid users shooting themselves in the foot.

Love it! Yes 👍

If users provide an end_date newer than 5 mins then I'd still automatically force the 5 mins buffer for safety. If we find cloudwatch docs explaining it with a more accurate SLA (likely in the seconds range vs mins) then I can shorten the hard coded buffer but I wouldnt want to give the user unsafe configuration options.

That's really smart. I hadn't really thought that far, but yes, I totally agree an enforced minimum offset sounds like the right thing to do.

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

No branches or pull requests

2 participants