SNOW-957349: Make some API/classes needed in stored procs public

[sfc-gh-smirzaei]

Overview

SNOW-957349

External contributors - please answer these questions before submitting a pull request. Thanks!

Please answer these questions before submitting your pull requests. Thanks!

1. What GitHub issue is this PR addressing? Make sure that there is an accompanying issue to your PR.

   Fixes #NNNN

2. Fill out the following pre-review checklist:

   • I am adding a new automated test(s) to verify correctness of my new code
   • I am adding new logging messages
   • I am modifying authorization mechanisms
   • I am adding new credentials
   • I am modifying OCSP code
   • I am adding a new dependency

3. Please describe how your code solves the related issue.
   As part of changes we made for enabling async child job in JVM stored procs, we needed to access some classes/APIs from the public library that are not public. Hence we had to create util methods in net/snowflake/client package (see this PR ...braries/jdbc-stored-proc/src/main/java/net/snowflake/client/core/StoredProcCoreUtils.java). This is a follow up change to make those classes/APIs public.

Can someone please point me to the precommit jenkins job that I need to run to test this?

Pre-review checklist

• This change has passed precommit
• I have reviewed code coverage report for my PR in (Sonarqube)

[sfc-gh-igarish]

@sfc-gh-smirzaei are we going to document this public types for any customers to use or just for internal purpose only? If it is only for internal purpose, please comment to propagate that message like "internal use only". Also check style merge gate failed so please run "mvn com.spotify.fmt:fmt-maven-plugin:format". This will format the code as needed.

[sfc-gh-dprzybysz]

@sfc-gh-smirzaei Please add issue id to the PR name and/or commit message

[sfc-gh-smirzaei]

@sfc-gh-smirzaei are we going to document this public types for any customers to use or just for internal purpose only? If it is only for internal purpose, please comment to propagate that message like "internal use only". Also check style merge gate failed so please run "mvn com.spotify.fmt:fmt-maven-plugin:format". This will format the code as needed.

We need this for internal use, but nothing is going to prevent these to be used publicly, so does it stillactually make sense to add "internal use only"? In general considering our workaround

@sfc-gh-smirzaei are we going to document this public types for any customers to use or just for internal purpose only? If it is only for internal purpose, please comment to propagate that message like "internal use only". Also check style merge gate failed so please run "mvn com.spotify.fmt:fmt-maven-plugin:format". This will format the code as needed.

I'm making this change for internal use in stored procs, but I assume nothing prevents public use-cases, so does it still make sense to add "internal use only" for something that is visible and accessible publicly?
In general considering our workaround for accessing these APIs/classes in this PR (https://github.com/snowflakedb/snowflake/pull/114773/files#diff-bb0f0d7b309b4b22ebeef522d0c556a4592aa80c198354099ee37ebd8551dae5), what do you think about this change? Do you see any downside?

Also thank you for the pointer for the formatting issue.

[sfc-gh-igarish]

"We need this for internal use, but nothing is going to prevent these to be used publicly, so does it stillactually >>make sense to add "internal use only"? In general considering our workaround"

As our source code is public, if we intended to use for internal purpose then better to tag that way, so if someone use and asking for support, we can say that please don't use or use it as per your own risk. Unless you think it will be useful for community to use this method. So it's more to protect from support side. We have seen that if public method use as intended it works but if many people start using it as non-intended purpose, we will be busy to fix and support it.

[sfc-gh-smirzaei]

"We need this for internal use, but nothing is going to prevent these to be used publicly, so does it stillactually >>make sense to add "internal use only"? In general considering our workaround"

As our source code is public, if we intended to use for internal purpose then better to tag that way, so if someone use and asking for support, we can say that please don't use or use it as per your own risk. Unless you think it will be useful for community to use this method. So it's more to protect from support side. We have seen that if public method use as intended it works but if many people start using it as non-intended purpose, we will be busy to fix and support it.

I see, good point. I'll add those comments.

[sfc-gh-smirzaei]

@sfc-gh-igarish the requested comment is added, please take a look once you get a chance.
On a separate note, do we have a jenkin job precommit I need to run before merging?

[sfc-gh-igarish]

LGTM

[sfc-gh-igarish]

On a separate note, do we have a jenkin job precommit I need to run before merging?

It will as part of this PR. We have JDBC-PC jobs run as part of each PR. So if all tests are running fine as github action, we are good. Monitor this github action check: "continuous-integration/jenkins/pr-merge" It shouldn't fail.

[sonarqubemergegate]

SonarQube Quality Gate

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information