From c57a9fd82bd59f7d7749e65fba7d955ba6cbe3b7 Mon Sep 17 00:00:00 2001 From: Toby Zhang Date: Fri, 12 Jan 2024 22:55:19 +0000 Subject: [PATCH 1/2] revert change --- .../ingest/connection/RequestBuilder.java | 82 ------------------- ...SnowflakeStreamingIngestClientFactory.java | 25 +----- ...nowflakeStreamingIngestClientInternal.java | 48 +---------- 3 files changed, 3 insertions(+), 152 deletions(-) diff --git a/src/main/java/net/snowflake/ingest/connection/RequestBuilder.java b/src/main/java/net/snowflake/ingest/connection/RequestBuilder.java index 869f734eb..605655c88 100644 --- a/src/main/java/net/snowflake/ingest/connection/RequestBuilder.java +++ b/src/main/java/net/snowflake/ingest/connection/RequestBuilder.java @@ -54,10 +54,6 @@ public class RequestBuilder { // whatever the actual host is private final String host; - private final String accountName; - - private final boolean addAccountNameInRequest; - private final String userAgentSuffix; // Reference to the telemetry service @@ -127,8 +123,6 @@ public class RequestBuilder { public static final String HTTP_HEADER_CONTENT_TYPE_JSON = "application/json"; - private static final String SF_HEADER_ACCOUNT_NAME = "Snowflake-Account"; - /** * RequestBuilder - general usage constructor * @@ -212,36 +206,6 @@ public RequestBuilder( null); } - /** - * RequestBuilder - constructor used by streaming ingest - * - * @param url - the Snowflake account to which we're connecting - * @param userName - the username of the entity loading files - * @param credential - the credential we'll use to authenticate - * @param httpClient - reference to the http client - * @param clientName - name of the client, used to uniquely identify a client if used - */ - public RequestBuilder( - SnowflakeURL url, - String userName, - Object credential, - CloseableHttpClient httpClient, - String clientName, - boolean addAccountNameInRequest) { - this( - url.getAccount(), - userName, - credential, - url.getScheme(), - url.getUrlWithoutPort(), - url.getPort(), - null, - null, - httpClient, - clientName, - addAccountNameInRequest); - } - /** * RequestBuilder - constructor used by streaming ingest * @@ -295,47 +259,6 @@ public RequestBuilder( SecurityManager securityManager, CloseableHttpClient httpClient, String clientName) { - this( - accountName, - userName, - credential, - schemeName, - hostName, - portNum, - userAgentSuffix, - securityManager, - httpClient, - clientName, - false); - } - - /** - * RequestBuilder - this constructor is for testing purposes only - * - * @param accountName - the account name to which we're connecting - * @param userName - for whom are we connecting? - * @param credential - our auth credentials, either JWT key pair or OAuth credential - * @param schemeName - are we HTTP or HTTPS? - * @param hostName - the host for this snowflake instance - * @param portNum - the port number - * @param userAgentSuffix - The suffix part of HTTP Header User-Agent - * @param securityManager - The security manager for authentication - * @param httpClient - reference to the http client - * @param clientName - name of the client, used to uniquely identify a client if used - * @param addAccountNameInRequest if ture, add account name in request header - */ - public RequestBuilder( - String accountName, - String userName, - Object credential, - String schemeName, - String hostName, - int portNum, - String userAgentSuffix, - SecurityManager securityManager, - CloseableHttpClient httpClient, - String clientName, - boolean addAccountNameInRequest) { // none of these arguments should be null if (accountName == null || userName == null || credential == null) { throw new IllegalArgumentException(); @@ -358,8 +281,6 @@ public RequestBuilder( this.scheme = schemeName; this.host = hostName; this.userAgentSuffix = userAgentSuffix; - this.accountName = accountName; - this.addAccountNameInRequest = addAccountNameInRequest; // create our security/token manager if (securityManager == null) { @@ -649,9 +570,6 @@ private static void addUserAgent(HttpUriRequest request, String userAgentSuffix) public void addToken(HttpUriRequest request) { request.setHeader(HttpHeaders.AUTHORIZATION, BEARER_PARAMETER + securityManager.getToken()); request.setHeader(SF_HEADER_AUTHORIZATION_TOKEN_TYPE, this.securityManager.getTokenType()); - if (addAccountNameInRequest) { - request.setHeader(SF_HEADER_ACCOUNT_NAME, accountName); - } } /** diff --git a/src/main/java/net/snowflake/ingest/streaming/SnowflakeStreamingIngestClientFactory.java b/src/main/java/net/snowflake/ingest/streaming/SnowflakeStreamingIngestClientFactory.java index ecab62432..2af794cd2 100644 --- a/src/main/java/net/snowflake/ingest/streaming/SnowflakeStreamingIngestClientFactory.java +++ b/src/main/java/net/snowflake/ingest/streaming/SnowflakeStreamingIngestClientFactory.java @@ -28,12 +28,6 @@ public static class Builder { // Allows client to override some default parameter values private Map parameterOverrides; - // preset SnowflakeURL instance - private SnowflakeURL snowflakeURL; - - // flag to specify if we need to add account name in the request header - private boolean addAccountNameInRequest; - private Builder(String name) { this.name = name; } @@ -43,16 +37,6 @@ public Builder setProperties(Properties prop) { return this; } - public Builder setSnowflakeURL(SnowflakeURL snowflakeURL) { - this.snowflakeURL = snowflakeURL; - return this; - } - - public Builder setAddAccountNameInRequest(boolean addAccountNameInRequest) { - this.addAccountNameInRequest = addAccountNameInRequest; - return this; - } - public Builder setParameterOverrides(Map parameterOverrides) { this.parameterOverrides = parameterOverrides; return this; @@ -63,15 +47,8 @@ public SnowflakeStreamingIngestClient build() { Utils.assertNotNull("connection properties", this.prop); Properties prop = Utils.createProperties(this.prop); - SnowflakeURL accountURL = this.snowflakeURL; - if (accountURL == null) { - accountURL = new SnowflakeURL(prop.getProperty(Constants.ACCOUNT_URL)); - } + SnowflakeURL accountURL = new SnowflakeURL(prop.getProperty(Constants.ACCOUNT_URL)); - if (addAccountNameInRequest) { - return new SnowflakeStreamingIngestClientInternal<>( - this.name, accountURL, prop, this.parameterOverrides, addAccountNameInRequest); - } return new SnowflakeStreamingIngestClientInternal<>( this.name, accountURL, prop, this.parameterOverrides); } diff --git a/src/main/java/net/snowflake/ingest/streaming/internal/SnowflakeStreamingIngestClientInternal.java b/src/main/java/net/snowflake/ingest/streaming/internal/SnowflakeStreamingIngestClientInternal.java index 47aa76d3b..55ade4d74 100644 --- a/src/main/java/net/snowflake/ingest/streaming/internal/SnowflakeStreamingIngestClientInternal.java +++ b/src/main/java/net/snowflake/ingest/streaming/internal/SnowflakeStreamingIngestClientInternal.java @@ -158,30 +158,6 @@ public class SnowflakeStreamingIngestClientInternal implements SnowflakeStrea boolean isTestMode, RequestBuilder requestBuilder, Map parameterOverrides) { - this(name, accountURL, prop, httpClient, isTestMode, requestBuilder, parameterOverrides, false); - } - - /** - * Constructor - * - * @param name the name of the client - * @param accountURL Snowflake account url - * @param prop connection properties - * @param httpClient http client for sending request - * @param isTestMode whether we're under test mode - * @param requestBuilder http request builder - * @param parameterOverrides parameters we override in case we want to set different values - * @param addAccountNameInRequest if true, will add account name in request header - */ - SnowflakeStreamingIngestClientInternal( - String name, - SnowflakeURL accountURL, - Properties prop, - CloseableHttpClient httpClient, - boolean isTestMode, - RequestBuilder requestBuilder, - Map parameterOverrides, - boolean addAccountNameInRequest) { this.parameterProvider = new ParameterProvider(parameterOverrides, prop); this.name = name; @@ -220,8 +196,7 @@ public class SnowflakeStreamingIngestClientInternal implements SnowflakeStrea prop.get(USER).toString(), credential, this.httpClient, - String.format("%s_%s", this.name, System.currentTimeMillis()), - addAccountNameInRequest); + String.format("%s_%s", this.name, System.currentTimeMillis())); logger.logInfo("Using {} for authorization", this.requestBuilder.getAuthType()); @@ -261,26 +236,7 @@ public SnowflakeStreamingIngestClientInternal( this(name, accountURL, prop, null, false, null, parameterOverrides); } - /** - * Default Constructor - * - * @param name the name of the client - * @param accountURL Snowflake account url - * @param prop connection properties - * @param parameterOverrides map of parameters to override for this client - * @param addAccountNameInRequest if true, add account name in request header - */ - public SnowflakeStreamingIngestClientInternal( - String name, - SnowflakeURL accountURL, - Properties prop, - Map parameterOverrides, - boolean addAccountNameInRequest) { - this(name, accountURL, prop, null, false, null, parameterOverrides, addAccountNameInRequest); - } - - /** - * Constructor for TEST ONLY + /*** Constructor for TEST ONLY * * @param name the name of the client */ From d24f8e147c2a7156d4caf35eeb262d6fbef98d0c Mon Sep 17 00:00:00 2001 From: Toby Zhang Date: Mon, 15 Jan 2024 12:59:42 -0800 Subject: [PATCH 2/2] NO-SNOW: fix test failures during releasing (#663) We're hitting test failures during snapshot release and the reason is because there's no profile.json file, this PR adds a test mode in SnowflakeStreamingIngestClientFactory so that we can continue use dummyHost name for unit testing. 2024-01-09 23:17:26 testClientFactorySuccess(net.snowflake.ingest.streaming.internal.SnowflakeStreamingIngestClientTest) Time elapsed: 180.565 sec <<< ERROR! 2024-01-09 23:17:26 net.snowflake.ingest.utils.SFException: Unable to connect to streaming ingest internal stage. 2024-01-09 23:17:26 at net.snowflake.ingest.streaming.internal.SnowflakeStreamingIngestClientTest.testClientFactorySuccess(SnowflakeStreamingIngestClientTest.java:266) 2024-01-09 23:17:26 Caused by: net.snowflake.client.jdbc.internal.apache.http.conn.ConnectTimeoutException: Connect to snowflake.qa1.int.snowflakecomputing.com:443 [snowflake.qa1.int.snowflakecomputing.com/10.180.20.15, snowflake.qa1.int.snowflakecomputing.com/10.180.20.17, snowflake.qa1.int.snowflakecomputing.com/10.180.20.16] failed: connect timed out 2024-01-09 23:17:26 at net.snowflake.ingest.streaming.internal.SnowflakeStreamingIngestClientTest.testClientFactorySuccess(SnowflakeStreamingIngestClientTest.java:266) 2024-01-09 23:17:26 Caused by: java.net.SocketTimeoutException: connect timed out 2024-01-09 23:17:26 at net.snowflake.ingest.streaming.internal.SnowflakeStreamingIngestClientTest.testClientFactorySuccess(SnowflakeStreamingIngestClientTest.java:266) 2024-01-09 23:17:26 2024-01-09 23:17:26 testConstructorParameters(net.snowflake.ingest.streaming.internal.SnowflakeStreamingIngestClientTest) Time elapsed: 180.392 sec <<< ERROR! --- .../SnowflakeStreamingIngestClientFactory.java | 10 +++++++++- .../SnowflakeStreamingIngestClientInternal.java | 6 ++++-- .../internal/SnowflakeStreamingIngestClientTest.java | 6 +++++- 3 files changed, 18 insertions(+), 4 deletions(-) diff --git a/src/main/java/net/snowflake/ingest/streaming/SnowflakeStreamingIngestClientFactory.java b/src/main/java/net/snowflake/ingest/streaming/SnowflakeStreamingIngestClientFactory.java index 2af794cd2..cd6d78787 100644 --- a/src/main/java/net/snowflake/ingest/streaming/SnowflakeStreamingIngestClientFactory.java +++ b/src/main/java/net/snowflake/ingest/streaming/SnowflakeStreamingIngestClientFactory.java @@ -28,6 +28,9 @@ public static class Builder { // Allows client to override some default parameter values private Map parameterOverrides; + // Indicates whether it's under test mode + private boolean isTestMode; + private Builder(String name) { this.name = name; } @@ -42,6 +45,11 @@ public Builder setParameterOverrides(Map parameterOverrides) { return this; } + public Builder setIsTestMode(boolean isTestMode) { + this.isTestMode = isTestMode; + return this; + } + public SnowflakeStreamingIngestClient build() { Utils.assertStringNotNullOrEmpty("client name", this.name); Utils.assertNotNull("connection properties", this.prop); @@ -50,7 +58,7 @@ public SnowflakeStreamingIngestClient build() { SnowflakeURL accountURL = new SnowflakeURL(prop.getProperty(Constants.ACCOUNT_URL)); return new SnowflakeStreamingIngestClientInternal<>( - this.name, accountURL, prop, this.parameterOverrides); + this.name, accountURL, prop, this.parameterOverrides, this.isTestMode); } } } diff --git a/src/main/java/net/snowflake/ingest/streaming/internal/SnowflakeStreamingIngestClientInternal.java b/src/main/java/net/snowflake/ingest/streaming/internal/SnowflakeStreamingIngestClientInternal.java index 55ade4d74..071f5f7c4 100644 --- a/src/main/java/net/snowflake/ingest/streaming/internal/SnowflakeStreamingIngestClientInternal.java +++ b/src/main/java/net/snowflake/ingest/streaming/internal/SnowflakeStreamingIngestClientInternal.java @@ -227,13 +227,15 @@ public class SnowflakeStreamingIngestClientInternal implements SnowflakeStrea * @param accountURL Snowflake account url * @param prop connection properties * @param parameterOverrides map of parameters to override for this client + * @param isTestMode indicates whether it's under test mode */ public SnowflakeStreamingIngestClientInternal( String name, SnowflakeURL accountURL, Properties prop, - Map parameterOverrides) { - this(name, accountURL, prop, null, false, null, parameterOverrides); + Map parameterOverrides, + boolean isTestMode) { + this(name, accountURL, prop, null, isTestMode, null, parameterOverrides); } /*** Constructor for TEST ONLY diff --git a/src/test/java/net/snowflake/ingest/streaming/internal/SnowflakeStreamingIngestClientTest.java b/src/test/java/net/snowflake/ingest/streaming/internal/SnowflakeStreamingIngestClientTest.java index cb75b3a52..0eece5f02 100644 --- a/src/test/java/net/snowflake/ingest/streaming/internal/SnowflakeStreamingIngestClientTest.java +++ b/src/test/java/net/snowflake/ingest/streaming/internal/SnowflakeStreamingIngestClientTest.java @@ -158,6 +158,7 @@ public void testConstructorParameters() throws Exception { SnowflakeStreamingIngestClientFactory.builder("client") .setProperties(prop) .setParameterOverrides(parameterMap) + .setIsTestMode(true) .build(); Assert.assertEquals("client", client.getName()); @@ -263,7 +264,10 @@ public void testClientFactorySuccess() throws Exception { prop.put(ROLE, TestUtils.getRole()); SnowflakeStreamingIngestClient client = - SnowflakeStreamingIngestClientFactory.builder("client").setProperties(prop).build(); + SnowflakeStreamingIngestClientFactory.builder("client") + .setProperties(prop) + .setIsTestMode(true) + .build(); Assert.assertEquals("client", client.getName()); Assert.assertFalse(client.isClosed());