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

Changing the Upload API to use a GET rather than a POST #110

Merged

Conversation

bryan-harter
Copy link
Member

Change Summary

Updating the API Gateway functions

Overview

Removed the "indexer" API, and changed the "upload" API to GET

New Dependencies

None

New Files

None

Deleted Files

None

Updated Files

  • api_gateway_stack.py
    • Removed the "indexer" API. The indexer lambda function triggers from new files arriving in the bucket, so it only gets invoked internally and doesn't need an API
    • Changed the upload from a "POST" to a "GET". The reason why it is a "GET" is because the upload API actually returns a signed url to the user to upload a data file.

Testing

@codecov
Copy link

codecov bot commented Jul 27, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (86446b6) 68.14% compared to head (1e664a2) 68.14%.
Report is 6 commits behind head on development.

Additional details and impacted files
@@             Coverage Diff              @@
##           development     #110   +/-   ##
============================================
  Coverage        68.14%   68.14%           
============================================
  Files               23       23           
  Lines              945      945           
============================================
  Hits               644      644           
  Misses             301      301           
Files Changed Coverage Δ
sds_data_manager/stacks/sds_data_manager_stack.py 32.50% <ø> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@greglucas greglucas left a comment

Choose a reason for hiding this comment

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

I agree this seems right. I assume that the Javascript SDK (i.e. eventual frontend website) uses a GET as well when interacting with this?

It looks like all methods are GET, do we anticipate anything else, otherwise we could get rid of the httpMethod here altogether and just make it a collection of tuples (endpoint_name, function)

@greglucas greglucas added this to the SDC SIT-2 milestone Jul 28, 2023
@greglucas greglucas added the Parent Req: Data Access API SOC-SDC-L4-59: Data Access API label Jul 28, 2023
@laspsandoval
Copy link
Contributor

looks good

@bryan-harter
Copy link
Member Author

I agree this seems right. I assume that the Javascript SDK (i.e. eventual frontend website) uses a GET as well when interacting with this?

It looks like all methods are GET, do we anticipate anything else, otherwise we could get rid of the httpMethod here altogether and just make it a collection of tuples (endpoint_name, function)

It's possible there could be post commands in the future so we might want to keep it. For example, if we do include a science impacting event database (where scientists can post solar events to a DB that shows up on the website, similar to the MAVEN SDC), that would be a POST. We might also make a POST that updates rows in opensearch, for example we could have an API that we use to handle public release process (setting status=public or public=True or something to the file in the opensearch instance)

@bryan-harter bryan-harter merged commit 712a9ea into IMAP-Science-Operations-Center:dev Aug 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Parent Req: Data Access API SOC-SDC-L4-59: Data Access API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants