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

Add implementation of delegating RequestStreamHandler #603

Merged
merged 10 commits into from
Jul 6, 2023

Conversation

olegz
Copy link
Collaborator

@olegz olegz commented Jun 28, 2023

This new handler delegates to spring-cloud-function-serverless-web module which is managed by Spring team

This PR is a substitution for the one I canceled #578 and is better in a way that it requires user to opt-in to use this handler. The current AWS code is not touched providing a very smooth and safe transition.
I will also be publishing a sample on how to use it

By submitting this pull request

  • I confirm that my contribution is made under the terms of the Apache 2.0 license.
  • I confirm that I've made a best effort attempt to update all relevant documentation.

@mbfreder
Copy link
Contributor

There are failing tests. @olegz Could you please take a look?

@olegz olegz force-pushed the spring-handler branch 2 times, most recently from 76394c0 to 123a1b4 Compare June 30, 2023 15:40
EndpointConfiguration: REGIONAL


Resources:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.

StageName contains invalid characters (Pattern: ^[a-zA-Z0-9-_]+$) at Resources/ServerlessHttpApiApiGatewayDefaultStage/Properties/StageName

Copy link
Contributor

@valerena valerena left a comment

Choose a reason for hiding this comment

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

It looks good other than the tabs that should be spaces.

@@ -29,10 +29,11 @@ public class AwsProxyRequest {
//-------------------------------------------------------------

private String body;
private String resource;
private String version;
private String resource;
Copy link
Contributor

Choose a reason for hiding this comment

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

Tab

Comment on lines 100 to 105
return version;
}

public void setVersion(String version) {
this.version = version;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Tabs

Comment on lines 290 to 306
<repository>
<id>spring-snapshots</id>
<name>Spring Snapshots</name>
<url>https://repo.spring.io/snapshot</url>
<snapshots>
<enabled>true</enabled>
</snapshots>
</repository>
<repository>
<id>spring-milestones</id>
<name>Spring Milestones</name>
<url>https://repo.spring.io/milestone</url>
<snapshots>
<enabled>false</enabled>
</snapshots>
</repository>
</repositories>
Copy link
Contributor

Choose a reason for hiding this comment

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

Tabs

*/
public class SpringDelegatingLambdaContainerHandler implements RequestStreamHandler {

private final Class<?>[] startupClasses;
Copy link
Contributor

Choose a reason for hiding this comment

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

Most of this file has tabs instead of spaces

@SuppressWarnings("rawtypes")
public class SpringDelegatingLambdaContainerHandlerTests {

private static String API_GATEWAY_EVENT = "{\n"
Copy link
Contributor

Choose a reason for hiding this comment

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

This whole file has tabs instead of spaces.


@Bean("CognitoIdentityFilter")
public Filter cognitoFilter() {
return new CognitoIdentityFilter();
Copy link
Contributor

Choose a reason for hiding this comment

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

There's a single tab here (4 spaces followed by a tab)

@olegz olegz force-pushed the spring-handler branch from baeaa1a to da5cb2b Compare July 6, 2023 08:31
@deki deki merged commit 430dcd0 into aws:main Jul 6, 2023
deki added a commit that referenced this pull request Jul 20, 2023
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.

5 participants