Skip to content

LocationLevel retrieve constant values in time window - #887

Closed
zack-rma wants to merge 5 commits into
USACE:developfrom
zack-rma:bugfix/location_level_issue_599
Closed

LocationLevel retrieve constant values in time window#887
zack-rma wants to merge 5 commits into
USACE:developfrom
zack-rma:bugfix/location_level_issue_599

Conversation

@zack-rma

@zack-rma zack-rma commented Sep 28, 2024

Copy link
Copy Markdown
Collaborator

Fixes #599, LocationLevel retrieve constant values in time window. Includes tests to verify functionality

@zack-rma
zack-rma marked this pull request as ready for review September 30, 2024 15:44
MikeNeilson
MikeNeilson previously approved these changes Sep 30, 2024
+ "specified), as well as the time zone of any times in the response."
+ " If this field is not specified, the default time zone of UTC "
+ "shall be used."),
@OpenApiParam(name = START, type = Instant.class, description = "Specifies the start of the time "

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Adding a start and end date for getOne does not make sense to me as it conflicts with the effective date parameter.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

That's fair, but because there are multiple effective dates there does need to be a way to retrieve them all without it necessarily being a "timeseries" as that's a different concept (seasonal, etc) of location level.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't see how that's different than the behavior of the getAll endpoint.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We'll see how Daniel responds.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@DanielTOsborne Following up for your thoughts on this

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@DanielTOsborne or @MikeNeilson Can we get a decision on this? If we can't decide what makes sense for getOne we could always not have a getOne and force users through getAll. I guess I'd like to see this merged and a new issue/pr opened to figure out next steps. People could try this out in the meantime

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Agreed. I believe people are already using the getOne though so I don't think we should remove it at this time. But PR itself should be appropriately updated and the work merged so we can move on.

If we don't get responses during development we're definitely going to get them after release.

@zack-rma

Copy link
Copy Markdown
Collaborator Author

Closing since branch is too out of date to reasonably merge. Superseded by PR: #1202

@zack-rma zack-rma closed this Jul 29, 2025
@zack-rma
zack-rma deleted the bugfix/location_level_issue_599 branch July 29, 2025 17:35
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.

Allow retrieval of constant value location levels over a date range

4 participants