Skip to content

Issue 6749: Rest API shell#6788

Draft
rtjd6554 wants to merge 19 commits intodevelopfrom
6749-rest-api-shell
Draft

Issue 6749: Rest API shell#6788
rtjd6554 wants to merge 19 commits intodevelopfrom
6749-rest-api-shell

Conversation

@rtjd6554
Copy link
Collaborator

Make sure you have checked all steps below.

Issue

  • My PR fully resolves the following issues. I've referenced an issue in the PR title, for example "Issue 1234 - My
    Feature". Note that before an issue is finished, you can still make a pull request by raising a separate issue
    for your progress.

Tests

  • My PR adds the following tests based on our test strategy OR does not need testing for this extremely good reason:
    • New tests within SleeperRestTest

Documentation

  • In case of new functionality, my PR adds documentation that describes how to use it, or I have linked to a
    separate issue for that below.
  • If I have added new Java code, I have added Javadoc that explains it following our conventions and style.
  • If I have added or removed any dependencies from the project, I have updated the NOTICES file.

Copy link
Collaborator

@patchwork01 patchwork01 left a comment

Choose a reason for hiding this comment

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

I think the idea of a basic endpoint to get the version number makes sense, but you need to decide what you want to implement to start with. You could start with the CDK code or the lambda code.

If you start with the CDK you'll need to focus on the CDK code to deploy API Gateway and hook it up to a lambda, and look up how to do that. You could add it to the cdk module as a new optional stack. The most similar existing code would be WebSocketQueryStack, which does use API Gateway but for a web socket.

If you start with the lambda you'll need to work out how to use the AWS libraries to write a handler that consumes the invocations from API Gateway. You'll need to work out whether it should be a single lambda per endpoint or a lambda that can serve multiple endpoints. The closest code that we've got already would be WebSocketQueryProcessorLambda, but that uses the events for web socket invocations, which is not what you want.

The configuration for how the lambda is invoked by API Gateway will be done in the CDK. I'd consider starting with that.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is configured as its own separate Maven project instead of as a module.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This doesn't look like it can be a lambda because it's got a main method and no handler method, and it doesn't look like a CDK app because you've written tests for it that read like you've making calls to a REST API.

Copy link
Collaborator

Choose a reason for hiding this comment

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

The tests here read like they'll test against a REST API, but I don't know if that'll exist as a testable thing. If it did I'm not sure it would make sense to group all the REST API tests together in one class, since we want it to be extensible.

We could test the lambda, and we could test the CDK app. For the REST API as a whole I think we'll be stuck with just system tests? We won't have a REST API until it's hooked up to API Gateway, so we'd need to deploy the system for that to work.

The tests here look as though they most fit as tests for a lambda that serves that one "get version" endpoint?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

REST API to configure Sleeper tables

2 participants