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

NO-SNOW: Revert one change that updates public API for internal use case #662

Merged
merged 3 commits into from
Jan 17, 2024

Conversation

sfc-gh-tzhang
Copy link
Contributor

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 #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.

Copy link
Collaborator

@sfc-gh-dwang sfc-gh-dwang left a comment

Choose a reason for hiding this comment

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

LGTM, I can create a new PR after POC if necessary

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!
Copy link
Contributor

@sfc-gh-xhuang sfc-gh-xhuang left a comment

Choose a reason for hiding this comment

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

thanks, best to remove before next sdk release

Copy link
Collaborator

@sfc-gh-japatel sfc-gh-japatel left a comment

Choose a reason for hiding this comment

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

lgtm!
Please have a meaning description in the title PR to not have to open commits from change logs.
Thanks

@sfc-gh-tzhang sfc-gh-tzhang changed the title NO-SNOW: Revert Change NO-SNOW: Revert one change that updates public API for internal use case Jan 17, 2024
@sfc-gh-tzhang sfc-gh-tzhang merged commit db1fd02 into master Jan 17, 2024
14 checks passed
@sfc-gh-tzhang sfc-gh-tzhang deleted the tzhang-si-revert branch January 17, 2024 08:27
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.

4 participants