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
Endpoints for PFT details & plotting of run outputs #2665
Conversation
if (nrow(qry_res) == 0) { | ||
PEcAn.DB::db.close(dbcon) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You might want to just use on.exit
to specify the db.close right when you open it to make sure that close happens even if any other sort of error occurs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah sure...that's a good idea...will try n incorporate this for all endpoints
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the problem is that the application does not exit, but stays open. We just need to make sure we close teh connections.
#* @get /<run_id>/graph/<year>/<y_var> | ||
#* @serializer contentType list(type='image/png') | ||
|
||
plotResults <- function(run_id, year, y_var, x_var="time", width=800, height=600, res){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not that there's anything 'wrong' with this, but I'd say that grabbing/downloading the model output itself is going to be FAR more important to most users than downloading an image file. Probably grabbing the whole netCDF would be the best place to start, but I could see a lot of folk wanting to subset variables, locations, data ranges, and ensemble members. The latter support it built into THREDDS, but (1) not all of our PEcAn servers support THREDDS (but maybe the right thing to do is to fix that) and (2) regardless we should have some helper functions in the R package to help people download output rather than having to learn to work with THREDDS directly (which is nontrivial)
I think there's also a REALLY high need, even if it's just internal to the project, to be able to correctly grab INPUTS and POSTERIORS via the API. The former will be pretty important to a lot of enduser analyses as well (e.g. plot predicted fluxes vs meteorological inputs)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function is needed in the current web application. The current web application can not create the graphs we had in the VM version. This will allow me/us to call this endpoint to get an image back.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As for the ability to download the outputs/inputs/posteriors I agree, we just have to find the right functions, but I think we have a lot of this framework in place.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahh, yes that makes perfect sense.
As we discussed during last week's planning meeting, we should think about whether it makes sense to rework the whole PHP interface to use the API rather than it's current approach of direct execution of R code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shhhhh, but that is my plan :) and maybe even use R and leaflet or other things.
/api/models/{model_id}
api/runs/{run_id}
if the outputs folder exists on the hostReview Time Estimate
Types of changes
Checklist: