-
Notifications
You must be signed in to change notification settings - Fork 59
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
SNOW-1357377 Add request Id in all streaming ingest APIs #759
Changes from 3 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -493,7 +493,6 @@ ChannelsStatusResponse getChannelsStatus( | |
.collect(Collectors.toList()); | ||
request.setChannels(requestDTOs); | ||
request.setRole(this.role); | ||
request.setRequestId(this.flushService.getClientPrefix() + "_" + counter.getAndIncrement()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should remove request_id in the request as well There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you mean in the POJO? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, in the request class |
||
|
||
String payload = objectMapper.writeValueAsString(request); | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,12 +1,7 @@ | ||
package net.snowflake.ingest.streaming.internal; | ||
|
||
import static java.time.ZoneOffset.UTC; | ||
import static net.snowflake.ingest.utils.Constants.ACCOUNT_URL; | ||
import static net.snowflake.ingest.utils.Constants.OPEN_CHANNEL_ENDPOINT; | ||
import static net.snowflake.ingest.utils.Constants.PRIVATE_KEY; | ||
import static net.snowflake.ingest.utils.Constants.RESPONSE_SUCCESS; | ||
import static net.snowflake.ingest.utils.Constants.ROLE; | ||
import static net.snowflake.ingest.utils.Constants.USER; | ||
import static net.snowflake.ingest.utils.Constants.*; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no star import |
||
import static org.mockito.ArgumentMatchers.argThat; | ||
|
||
import java.security.KeyPair; | ||
|
@@ -313,8 +308,14 @@ public void testOpenChannelPostRequest() throws Exception { | |
requestBuilder.generateStreamingIngestPostRequest( | ||
payload, OPEN_CHANNEL_ENDPOINT, "open channel"); | ||
|
||
Assert.assertEquals( | ||
String.format("%s%s", urlStr, OPEN_CHANNEL_ENDPOINT), request.getRequestLine().getUri()); | ||
String expectedUrlPattern = | ||
String.format("%s%s", urlStr, OPEN_CHANNEL_ENDPOINT) + "(\\?requestId=[a-f0-9\\-]{36})?"; | ||
|
||
Assert.assertTrue( | ||
String.format( | ||
"Expected URL to match pattern: %s but was: %s", | ||
expectedUrlPattern, request.getRequestLine().getUri()), | ||
request.getRequestLine().getUri().matches(expectedUrlPattern)); | ||
Assert.assertNotNull(request.getFirstHeader(HttpHeaders.USER_AGENT)); | ||
Assert.assertNotNull(request.getFirstHeader(HttpHeaders.AUTHORIZATION)); | ||
Assert.assertEquals("POST", request.getMethod()); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My assumption is that the request id will remain the same with retry?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thats true!