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

Feature/ForecastControllers #591

Merged
merged 10 commits into from
Apr 9, 2024

Conversation

dreina-gei
Copy link
Contributor

@dreina-gei dreina-gei commented Mar 20, 2024

Controllers for forecast data

@dreina-gei
Copy link
Contributor Author

Some questions regarding forecast spec. For the getAll request should the officeId parameter have a filter or should it be included as a single officeId? Furthermore, is the officeId a required parameter? Would a source entity filter be useful?

@MikeNeilson
Copy link
Contributor

For getAll probably not required. but certainly useful for filtering.
Source entity, name, and designator are probably the most other useful.

cwms-data-api/src/main/java/cwms/cda/api/Controllers.java Outdated Show resolved Hide resolved
cwms-data-api/src/main/java/cwms/cda/api/Controllers.java Outdated Show resolved Hide resolved
@@ -121,15 +121,22 @@ public final class Controllers {
public static final String TIMESERIES = "timeseries";
public static final String LOCATIONS = "locations";

public static final String SPEC_ID = "spec-id";
public static final String LOCATION = "location";
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd go with "location-id", surprised there isn't an example already

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Collaborator

Choose a reason for hiding this comment

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

you can remove this one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

tags = TAG
)
@Override
public void create(@NotNull Context ctx) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm re-thinking how this create would work.

Workflow:

client creates a spec, providing a list of time series ids
client creates an instance for a spec when going to run a forecast
client posts the results for the list of time series associated with the spec. -> isn't this just the time series controller? the spec is already associated with the time series identifier and the instance is already associated with the values given the forecast date and the time series id. What extra logic is needed by CDA to tie the time series storage to the instance?

Similarly for the delete, any interactions would be in the existing timeseries controller.

@dreina-gei dreina-gei requested a review from adamkorynta March 29, 2024 21:30
Copy link
Collaborator

Choose a reason for hiding this comment

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

This file shouldn't have needed to change

@rma-rripken rma-rripken merged commit c212b7e into USACE:develop Apr 9, 2024
3 checks passed
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