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

CWMVUE-634 Implements Access To Water Endpoint #1030

Draft
wants to merge 7 commits into
base: develop
Choose a base branch
from

Conversation

rma-bryson
Copy link
Collaborator

No description provided.

@rma-bryson rma-bryson force-pushed the feature/Adding_Access_To_Water_Endpoint branch from 0670b66 to 1d4ecf3 Compare February 25, 2025 22:51
Copy link
Collaborator

@adamkorynta adamkorynta left a comment

Choose a reason for hiding this comment

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

"type" is a bit ambiguous, especially that is is describing the type of parameter (but not to be confused with parameter type inst/ave/const/etc). otherwise the structure looks good. I'd probably prefer parameter even if it doesn't exactly match the parameter in the time series identifier.

@rma-bryson rma-bryson force-pushed the feature/Adding_Access_To_Water_Endpoint branch from 41be4ec to 4950f40 Compare February 27, 2025 17:08
@MikeNeilson
Copy link
Contributor

I don't hate it, but it's not giving me any warm and fuzzies for the future either.

@jeffsuperglide, @adamscarberry can you grant @rma-bryson and @adamkorynta at least read access to the access to water repositories to see how you're using the various A2W tables. I don't recall the usage being super high. But since this particular interface and objects will be new to everyone it's a good time to at least consider changes to how the data is presented and I'm not involved enough on the A2W side to say if the proposed design will be sensible.

It may just be that these new endpoints are only used for the district level control of things and it may not matter, but better to get a conversation started sooner rather than later.

@rma-bryson I would suggest also considering the following scenario to provide a little more insight:

  • 3 dams from different districts
  • at least 3 different elements of data for each.

The current example and test has one, which isn't a good sample size for something that could be rather foundational to our various operations.

@adamscarberry
Copy link

…sts and further updates are needed here, but pausing work for now.
@rma-bryson rma-bryson marked this pull request as draft March 6, 2025 18:46
@rma-bryson rma-bryson changed the title CWMVUE-634 Implements Access To Water DTO CWMVUE-634 Implements Access To Water Endpoint Mar 6, 2025
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.

4 participants