-
Notifications
You must be signed in to change notification settings - Fork 5
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
chore(api): scenario cost surfaces shell #174
Conversation
This pull request is being automatically deployed with Vercel (learn more). marxan – ./app🔍 Inspect: https://vercel.com/vizzuality1/marxan/FKPKyAo9rzG29zjvap36nBufq3Za marxan-storybook – ./app🔍 Inspect: https://vercel.com/vizzuality1/marxan-storybook/B7Jy7PuvtSFF4VFkNEwww45BHGue |
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.
@kgajowy thank you. The shell looks ok. I would still prefer to have geo-jobs be self-contained in the geoprocessing service, especially where we don't need to send back anything to the API client, but between ApiEvents and other bits, it's true that there is quite some code we'd need to share or (heavens forbid) duplicate between API and geoprocessing service.
In this case, maybe we could simplify the process a bit by handling just a bit more than shp->geojson transformation in the geoprocessing service: we'll only need the id of each planning unit and the associated cost, so we can get the geoprocessing service to return that, and get back a (potentially) much smaller payload
geo: { | ||
features: [], | ||
type: 'FeatureCollection', | ||
}, |
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 could probably be a plain list of ids and associated cost value
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 would refrain from putting more logic there. We already have analysis module
which handles validation of integrity and persistence - the facade is "only" connecting those two pieces together (i.e. rest->shapefile->geo->process)
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.
Revisited in 1a12fbb
8aa1df6
to
7e0df0b
Compare
1a12fbb
to
1d83fa5
Compare
c01b756
to
6455cea
Compare
Proposal of flow/structures for cost surface shapefile upload
Overview
Please write a description. If the PR is hard to understand, provide a quick explanation of the code.
Designs
Link to the related design prototypes (if applicable)
Testing instructions
Please explain how to test the PR: ID of a dataset, steps to reach the feature, etc.
Feature relevant tickets
https://vizzuality.atlassian.net/browse/MARXAN-291
Checklist before submitting
develop
.deploying to staging/production, please add brief testing instructions
to the deploy checklist (
docs/deployment-checklist.md
)