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

Added endpoint to the API for universal spin table #199

Merged

Conversation

tech3371
Copy link
Contributor

@tech3371 tech3371 commented Dec 1, 2023

Change Summary

Create endpoint for spin table query. Updated how we add route to the API and updated places that uses it.

New Dependencies

New Files

  • sds_data_manager/lambda_code/SDSCode/spin_table_api.py
    • Place holder for API handler code

Updated Files

  • sds_data_manager/stacks/api_gateway_stack.py
    • added stack to create Lambda that handler API calls.
    • Refactor code to be able to add route easily.
  • sds_data_manager/stacks/database_stack.py and sds_data_manager/stacks/domain_stack.py and sds_data_manager/stacks/ecr_stack.py
    • Added comments for future use
  • sds_data_manager/stacks/sds_data_manager_stack.py
    • Updated to use new way to add route
  • sds_data_manager/utils/stackbuilder.py
    • Rearrange stack to create resources it needs
    • Added new stack to create spin table endpoint

closes IMAP-Science-Operations-Center/imap_processing#243

@tech3371 tech3371 added this to the SDC SIT-3 milestone Dec 1, 2023
@tech3371 tech3371 requested a review from a team December 1, 2023 21:12
@tech3371 tech3371 self-assigned this Dec 1, 2023
@tech3371 tech3371 requested review from bourque, sdhoyt, greglucas, bryan-harter, laspsandoval, GFMoraga and maxinelasp and removed request for a team December 1, 2023 21:12
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.

Looks like the tests also need to be updated, but overall looks like a nice abstraction to me!

lambda_code_directory = Path(__file__).parent.parent / "lambda_code" / "SDSCode"
lambda_code_directory_str = str(lambda_code_directory.resolve())

spin_table_code = lambda_code_directory / "spin_table_api.py"
# Create Lambda for universal spin table API
spin_spin_api_handler = api_gateway_stack.APILambda(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
spin_spin_api_handler = api_gateway_stack.APILambda(
spin_api_stack = api_gateway_stack.APILambda(

All the other routes live in the sds_data_manager stack. Should this be moved up there too? It kind of sticks out as the only Lambda at this level right now. But maybe we can reorganize again in a follow-up PR...

Copy link
Contributor Author

@tech3371 tech3371 Dec 1, 2023

Choose a reason for hiding this comment

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

I think future PR because I would like to group API lambda out. But I wanted to wait until other PR is merged.

greglucas
greglucas previously approved these changes Dec 1, 2023
Copy link
Collaborator

@bourque bourque left a comment

Choose a reason for hiding this comment

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

Nice work! I just have a few non-blocking comments/suggestions



def lambda_handler(event, context):
print(event)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I reckon that you have this here for debugging purposes? Could it be removed?

@@ -0,0 +1,8 @@
import json
Copy link
Collaborator

Choose a reason for hiding this comment

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

I've been bugging everyone to add module docstrings so I will do the same here 😅

def lambda_template():
app = cdk.App()
lambda_code_directory = (
Path(__file__).parent.parent.parent / "sds_data_manager/lambda_code/SDSCode"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Perhaps we could define the top-level directory in the __init__.py file and import/use it from there, like we do in the imap_processing repo: https://github.com/IMAP-Science-Operations-Center/imap_processing/blob/59b42d30453b769455e943ca9913a2cd1317f27a/imap_processing/__init__.py#L10

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that's a great idea. Do you mind if I do this and function doc string in the upcoming PR?

@tech3371 tech3371 merged commit 0492806 into IMAP-Science-Operations-Center:dev Dec 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Universal Spin Table API Endpoint
3 participants