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

Add start datetime parameter to gsp/pvlive endpoint #255

Merged
merged 8 commits into from
Aug 28, 2023

Conversation

devsjc
Copy link
Contributor

@devsjc devsjc commented Jul 28, 2023

Adds the start_datetime_utc parameter to gsp/pvlive to enable specification of a start datetime

@devsjc
Copy link
Contributor Author

devsjc commented Jul 28, 2023

@braddf @rachel-labri-tipton Only thing I'm not sure about here is, if we're setting the usual number of days back that this endpoing can access behind an environment variable on the server side, perhaps there was the intention to keep whatever we set as N_HISTORY_DAYS as the maximum that a client-side user could ask for? This would overwrite that - so maybe I should put a block in to limit it? Or remove start datetime as an option and just change it to something like "snapshot" as a bool which just sets the startdate to last 15 minutes, preventing abuse?

@devsjc, I went ahead and ran the tests on this and the route looks good.
I could be missing something, but how is having a "snapshot" (which is better named that "historic") different from having the "historic" bool?

@devsjc
Copy link
Contributor Author

devsjc commented Jul 28, 2023

@devsjc, I went ahead and ran the tests on this and the route looks good.
I could be missing something, but how is having a "snapshot" (which is better named that "historic") different from having the "historic" bool?

It might not be much better! It could just be better than target_datetime as an option as it would prevent a user trying to access the entire contents of the table with one call via a very early start_datetime_utc.

@peterdudfield
Copy link
Collaborator

@devsjc, I went ahead and ran the tests on this and the route looks good.
I could be missing something, but how is having a "snapshot" (which is better named that "historic") different from having the "historic" bool?

It might not be much better! It could just be better than target_datetime as an option as it would prevent a user trying to access the entire contents of the table with one call via a very early start_datetime_utc.

I like the idea of this, I would perhaps put the limit on, so that the user can get the whole history. Thats what PVlive is for. Could you do this @devsjc ?

Copy link
Collaborator

@peterdudfield peterdudfield left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add limit, so can only pull at max 3 days of data

@devsjc devsjc requested a review from peterdudfield August 23, 2023 09:40
@peterdudfield
Copy link
Collaborator

This helps with #269

@peterdudfield peterdudfield merged commit 797ce23 into main Aug 28, 2023
@peterdudfield peterdudfield deleted the gsp-pvlive-lesshistory branch August 28, 2023 20:21
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 this pull request may close these issues.

3 participants