914 [Feature] - "Add remaining CRUD endpoints to SST"

[alantao912]

Added remaining trainspace CRUD endpoints to SST

What user problem are we solving? Trainspace creation, deletion, management, and deletion of user uploaded datset.

What solution does this PR provide? Allowing users to create new, retrieve specific, retrieve all, delete specific trainspace, and delete specific user uploaded dataset using AWS Lambda.

Testing Methodology Tested API endpoints on locally with postman using a developer account.

Created new trainspace, retrieved it, created several new trainspaces, retrieved all, deleted trainspace, attempted (and successfully failed) to retrieve previously deleted trainspace, and deleted user uploaded data successfully.

Any other considerations

• More thorough testing may be needed for deleting datasets for S3 buckets, haven't checked false negative case. No exceptions were thrown however.
• May need to improve exception messages, and exception detection.

[alantao912]

Sept 23rd. Latest

[alantao912]

also, can you attach a screen recording to show that the api endpoint works and data is updated on trainspace dynamodb correctly?

https://youtu.be/aMiK-I8cpqs

[karkir0003]

@alantao912 if you've addressed all the PR comments and tested your changes using the method outlined in your video, let me know and I can mark your PR as approved to unblock

[karkir0003]

@alantao912 there are some merge conflicts to be resolved?

[karkir0003]

@alantao912 can you fix the merge conflicts and also test the endpoints?

[alantao912]

@alantao912 can you fix the merge conflicts and also test the endpoints?

No merge conflicts. Fast-forwarded and pushed.

[karkir0003]

@alantao912 can you fix the merge conflicts and also test the endpoints?

No merge conflicts. Fast-forwarded and pushed.

did you test the endpoints @alantao912

[alantao912]

@karkir0003 Hey, narrowed permissions on the SST endpoints and tested with the same method as before. Should be good to go.

[karkir0003]

I think it would be better if each trainspace type was handled by it's own crud api endpoints rather than having a conglomerate trainspace crud endpoints. Our current way of approaching this seems to be leading to the massive switch statement, giant functions, etc, and from looking at the current implementation, it seems like each trainspace type seems to have fundamentally different needs when it comes to implementing the CRUD endpoints for them. I think a CRUD api endpoint set each for tabular, image, classical, etc would be better. @karkir0003 what do you think

fair point @dwu359 but what would the diff between these crud endpoints look like?

[dwu359]

I think it would be better if each trainspace type was handled by it's own crud api endpoints rather than having a conglomerate trainspace crud endpoints. Our current way of approaching this seems to be leading to the massive switch statement, giant functions, etc, and from looking at the current implementation, it seems like each trainspace type seems to have fundamentally different needs when it comes to implementing the CRUD endpoints for them. I think a CRUD api endpoint set each for tabular, image, classical, etc would be better. @karkir0003 what do you think

fair point @dwu359 but what would the diff between these crud endpoints look like?

A set of CRUD endpoints for tabular, a set of CRUD endpoints for image, a set of CRUD endpoints for classical ml. Basically just a separation of concerns

[karkir0003]

like treat trainspace types differently? dont we already segregate tabular train from say image train

[karkir0003]

@alantao912 thoughts?

[karkir0003]

what makes img trainspace different from say a tabular trainspace?

[dwu359]

Yes. Although all trainspaces are still stored in the same trainspace table. We would still need a get all trainspaces endpoint for the dashboard and a delete trainspace by id for deleting from the dashboard. So I guess it's just the create endpoints that need to get separated then

[dwu359]

what makes img trainspace different from say a tabular trainspace?

Mainly different json structures, which may require different ways of handling such data

[alantao912]

@alantao912 thoughts?

I don't think there would need to be a complete set of new CRUD endpoints for each type of trainspace. There only need to be different handling for creating new trainspaces. Deletion, reading, should not vary between data sources.

[farisdurrani]

You're quite behind the nextjs branch. Just fix the conflicts and you should be good to go 👍 @alantao912

[sonarcloud[bot]]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication