Skip to content

Commit c2c4dd2

Browse files
sfc-gh-tzhangsangeet259
authored andcommitted
NO-SNOW: Revert one change that updates public API for internal use case (snowflakedb#662)
Revert 2b702db This change updates the public API for an internal special use case only which is not ideal, looks like we keep updating public APIs in snowflakedb#661 as well. Given that we're still in a POC phase, I suggest we do everything in another branch instead, hence removing this from the next public release and could discuss what's the best way to support this internal use case once the POC is done.
1 parent 11a18e7 commit c2c4dd2

File tree

4 files changed

+18
-153
lines changed

4 files changed

+18
-153
lines changed

src/main/java/net/snowflake/ingest/connection/RequestBuilder.java

Lines changed: 0 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -54,10 +54,6 @@ public class RequestBuilder {
5454
// whatever the actual host is
5555
private final String host;
5656

57-
private final String accountName;
58-
59-
private final boolean addAccountNameInRequest;
60-
6157
private final String userAgentSuffix;
6258

6359
// Reference to the telemetry service
@@ -127,8 +123,6 @@ public class RequestBuilder {
127123

128124
public static final String HTTP_HEADER_CONTENT_TYPE_JSON = "application/json";
129125

130-
private static final String SF_HEADER_ACCOUNT_NAME = "Snowflake-Account";
131-
132126
/**
133127
* RequestBuilder - general usage constructor
134128
*
@@ -212,36 +206,6 @@ public RequestBuilder(
212206
null);
213207
}
214208

215-
/**
216-
* RequestBuilder - constructor used by streaming ingest
217-
*
218-
* @param url - the Snowflake account to which we're connecting
219-
* @param userName - the username of the entity loading files
220-
* @param credential - the credential we'll use to authenticate
221-
* @param httpClient - reference to the http client
222-
* @param clientName - name of the client, used to uniquely identify a client if used
223-
*/
224-
public RequestBuilder(
225-
SnowflakeURL url,
226-
String userName,
227-
Object credential,
228-
CloseableHttpClient httpClient,
229-
String clientName,
230-
boolean addAccountNameInRequest) {
231-
this(
232-
url.getAccount(),
233-
userName,
234-
credential,
235-
url.getScheme(),
236-
url.getUrlWithoutPort(),
237-
url.getPort(),
238-
null,
239-
null,
240-
httpClient,
241-
clientName,
242-
addAccountNameInRequest);
243-
}
244-
245209
/**
246210
* RequestBuilder - constructor used by streaming ingest
247211
*
@@ -295,47 +259,6 @@ public RequestBuilder(
295259
SecurityManager securityManager,
296260
CloseableHttpClient httpClient,
297261
String clientName) {
298-
this(
299-
accountName,
300-
userName,
301-
credential,
302-
schemeName,
303-
hostName,
304-
portNum,
305-
userAgentSuffix,
306-
securityManager,
307-
httpClient,
308-
clientName,
309-
false);
310-
}
311-
312-
/**
313-
* RequestBuilder - this constructor is for testing purposes only
314-
*
315-
* @param accountName - the account name to which we're connecting
316-
* @param userName - for whom are we connecting?
317-
* @param credential - our auth credentials, either JWT key pair or OAuth credential
318-
* @param schemeName - are we HTTP or HTTPS?
319-
* @param hostName - the host for this snowflake instance
320-
* @param portNum - the port number
321-
* @param userAgentSuffix - The suffix part of HTTP Header User-Agent
322-
* @param securityManager - The security manager for authentication
323-
* @param httpClient - reference to the http client
324-
* @param clientName - name of the client, used to uniquely identify a client if used
325-
* @param addAccountNameInRequest if ture, add account name in request header
326-
*/
327-
public RequestBuilder(
328-
String accountName,
329-
String userName,
330-
Object credential,
331-
String schemeName,
332-
String hostName,
333-
int portNum,
334-
String userAgentSuffix,
335-
SecurityManager securityManager,
336-
CloseableHttpClient httpClient,
337-
String clientName,
338-
boolean addAccountNameInRequest) {
339262
// none of these arguments should be null
340263
if (accountName == null || userName == null || credential == null) {
341264
throw new IllegalArgumentException();
@@ -358,8 +281,6 @@ public RequestBuilder(
358281
this.scheme = schemeName;
359282
this.host = hostName;
360283
this.userAgentSuffix = userAgentSuffix;
361-
this.accountName = accountName;
362-
this.addAccountNameInRequest = addAccountNameInRequest;
363284

364285
// create our security/token manager
365286
if (securityManager == null) {
@@ -649,9 +570,6 @@ private static void addUserAgent(HttpUriRequest request, String userAgentSuffix)
649570
public void addToken(HttpUriRequest request) {
650571
request.setHeader(HttpHeaders.AUTHORIZATION, BEARER_PARAMETER + securityManager.getToken());
651572
request.setHeader(SF_HEADER_AUTHORIZATION_TOKEN_TYPE, this.securityManager.getTokenType());
652-
if (addAccountNameInRequest) {
653-
request.setHeader(SF_HEADER_ACCOUNT_NAME, accountName);
654-
}
655573
}
656574

657575
/**

src/main/java/net/snowflake/ingest/streaming/SnowflakeStreamingIngestClientFactory.java

Lines changed: 8 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,8 @@ public static class Builder {
2828
// Allows client to override some default parameter values
2929
private Map<String, Object> parameterOverrides;
3030

31-
// preset SnowflakeURL instance
32-
private SnowflakeURL snowflakeURL;
33-
34-
// flag to specify if we need to add account name in the request header
35-
private boolean addAccountNameInRequest;
31+
// Indicates whether it's under test mode
32+
private boolean isTestMode;
3633

3734
private Builder(String name) {
3835
this.name = name;
@@ -43,18 +40,13 @@ public Builder setProperties(Properties prop) {
4340
return this;
4441
}
4542

46-
public Builder setSnowflakeURL(SnowflakeURL snowflakeURL) {
47-
this.snowflakeURL = snowflakeURL;
48-
return this;
49-
}
50-
51-
public Builder setAddAccountNameInRequest(boolean addAccountNameInRequest) {
52-
this.addAccountNameInRequest = addAccountNameInRequest;
43+
public Builder setParameterOverrides(Map<String, Object> parameterOverrides) {
44+
this.parameterOverrides = parameterOverrides;
5345
return this;
5446
}
5547

56-
public Builder setParameterOverrides(Map<String, Object> parameterOverrides) {
57-
this.parameterOverrides = parameterOverrides;
48+
public Builder setIsTestMode(boolean isTestMode) {
49+
this.isTestMode = isTestMode;
5850
return this;
5951
}
6052

@@ -63,17 +55,10 @@ public SnowflakeStreamingIngestClient build() {
6355
Utils.assertNotNull("connection properties", this.prop);
6456

6557
Properties prop = Utils.createProperties(this.prop);
66-
SnowflakeURL accountURL = this.snowflakeURL;
67-
if (accountURL == null) {
68-
accountURL = new SnowflakeURL(prop.getProperty(Constants.ACCOUNT_URL));
69-
}
58+
SnowflakeURL accountURL = new SnowflakeURL(prop.getProperty(Constants.ACCOUNT_URL));
7059

71-
if (addAccountNameInRequest) {
72-
return new SnowflakeStreamingIngestClientInternal<>(
73-
this.name, accountURL, prop, this.parameterOverrides, addAccountNameInRequest);
74-
}
7560
return new SnowflakeStreamingIngestClientInternal<>(
76-
this.name, accountURL, prop, this.parameterOverrides);
61+
this.name, accountURL, prop, this.parameterOverrides, this.isTestMode);
7762
}
7863
}
7964
}

src/main/java/net/snowflake/ingest/streaming/internal/SnowflakeStreamingIngestClientInternal.java

Lines changed: 5 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -158,30 +158,6 @@ public class SnowflakeStreamingIngestClientInternal<T> implements SnowflakeStrea
158158
boolean isTestMode,
159159
RequestBuilder requestBuilder,
160160
Map<String, Object> parameterOverrides) {
161-
this(name, accountURL, prop, httpClient, isTestMode, requestBuilder, parameterOverrides, false);
162-
}
163-
164-
/**
165-
* Constructor
166-
*
167-
* @param name the name of the client
168-
* @param accountURL Snowflake account url
169-
* @param prop connection properties
170-
* @param httpClient http client for sending request
171-
* @param isTestMode whether we're under test mode
172-
* @param requestBuilder http request builder
173-
* @param parameterOverrides parameters we override in case we want to set different values
174-
* @param addAccountNameInRequest if true, will add account name in request header
175-
*/
176-
SnowflakeStreamingIngestClientInternal(
177-
String name,
178-
SnowflakeURL accountURL,
179-
Properties prop,
180-
CloseableHttpClient httpClient,
181-
boolean isTestMode,
182-
RequestBuilder requestBuilder,
183-
Map<String, Object> parameterOverrides,
184-
boolean addAccountNameInRequest) {
185161
this.parameterProvider = new ParameterProvider(parameterOverrides, prop);
186162

187163
this.name = name;
@@ -220,8 +196,7 @@ public class SnowflakeStreamingIngestClientInternal<T> implements SnowflakeStrea
220196
prop.get(USER).toString(),
221197
credential,
222198
this.httpClient,
223-
String.format("%s_%s", this.name, System.currentTimeMillis()),
224-
addAccountNameInRequest);
199+
String.format("%s_%s", this.name, System.currentTimeMillis()));
225200

226201
logger.logInfo("Using {} for authorization", this.requestBuilder.getAuthType());
227202

@@ -252,35 +227,18 @@ public class SnowflakeStreamingIngestClientInternal<T> implements SnowflakeStrea
252227
* @param accountURL Snowflake account url
253228
* @param prop connection properties
254229
* @param parameterOverrides map of parameters to override for this client
255-
*/
256-
public SnowflakeStreamingIngestClientInternal(
257-
String name,
258-
SnowflakeURL accountURL,
259-
Properties prop,
260-
Map<String, Object> parameterOverrides) {
261-
this(name, accountURL, prop, null, false, null, parameterOverrides);
262-
}
263-
264-
/**
265-
* Default Constructor
266-
*
267-
* @param name the name of the client
268-
* @param accountURL Snowflake account url
269-
* @param prop connection properties
270-
* @param parameterOverrides map of parameters to override for this client
271-
* @param addAccountNameInRequest if true, add account name in request header
230+
* @param isTestMode indicates whether it's under test mode
272231
*/
273232
public SnowflakeStreamingIngestClientInternal(
274233
String name,
275234
SnowflakeURL accountURL,
276235
Properties prop,
277236
Map<String, Object> parameterOverrides,
278-
boolean addAccountNameInRequest) {
279-
this(name, accountURL, prop, null, false, null, parameterOverrides, addAccountNameInRequest);
237+
boolean isTestMode) {
238+
this(name, accountURL, prop, null, isTestMode, null, parameterOverrides);
280239
}
281240

282-
/**
283-
* Constructor for TEST ONLY
241+
/*** Constructor for TEST ONLY
284242
*
285243
* @param name the name of the client
286244
*/

src/test/java/net/snowflake/ingest/streaming/internal/SnowflakeStreamingIngestClientTest.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,7 @@ public void testConstructorParameters() throws Exception {
158158
SnowflakeStreamingIngestClientFactory.builder("client")
159159
.setProperties(prop)
160160
.setParameterOverrides(parameterMap)
161+
.setIsTestMode(true)
161162
.build();
162163

163164
Assert.assertEquals("client", client.getName());
@@ -263,7 +264,10 @@ public void testClientFactorySuccess() throws Exception {
263264
prop.put(ROLE, TestUtils.getRole());
264265

265266
SnowflakeStreamingIngestClient client =
266-
SnowflakeStreamingIngestClientFactory.builder("client").setProperties(prop).build();
267+
SnowflakeStreamingIngestClientFactory.builder("client")
268+
.setProperties(prop)
269+
.setIsTestMode(true)
270+
.build();
267271

268272
Assert.assertEquals("client", client.getName());
269273
Assert.assertFalse(client.isClosed());

0 commit comments

Comments
 (0)