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

Amazon Lambda - minor cleanup #2197

Merged
merged 1 commit into from
Apr 25, 2019
Merged

Conversation

mkouba
Copy link
Contributor

@mkouba mkouba commented Apr 24, 2019

No description provided.

@mkouba mkouba force-pushed the aws-lambda-cleanup branch 2 times, most recently from 68bb1f4 to 52b2cfb Compare April 24, 2019 12:47
@mkouba mkouba requested a review from evanchooly April 24, 2019 12:48
HttpURLConnection responseConnection = (HttpURLConnection) responseUrl.openConnection();
responseConnection.setDoOutput(true);
responseConnection.setRequestMethod("POST");
mapper.writeValue(responseConnection.getOutputStream(), response);
while (responseConnection.getInputStream().read() != -1) {

// Read data
Copy link
Member

Choose a reason for hiding this comment

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

what's happening here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, I think it reads the data until the end of the stream is reached. I've just added a comment because an empty while loop looks weird to me ;-).

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@mkouba mkouba force-pushed the aws-lambda-cleanup branch from 52b2cfb to 1a72573 Compare April 24, 2019 19:44
public void handle(HttpServerExchange exchange, String message) {
ObjectMapper mapper = new ObjectMapper();
try {
FunctionError result = mapper.readerFor(FunctionError.class).readValue(message);
Copy link
Member

Choose a reason for hiding this comment

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

I think we should cache the reader here. It can be done in another PR though as it was already preexisting. I don't know if we use this pattern elsewhere in the code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If it is safe to share a reader then +1 for a new PR ;-)

Copy link
Member

Choose a reason for hiding this comment

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

Ah, I missed it's just in the tests. We don't really care then.

@gsmet gsmet merged commit e3c431b into quarkusio:master Apr 25, 2019
@gsmet gsmet added this to the 0.15.0 milestone Apr 25, 2019
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.

None yet

3 participants