Conversation
|
⏳ I'm reviewing this pull request for security vulnerabilities and code quality issues. I'll provide an update when I'm done |
1 similar comment
|
⏳ I'm reviewing this pull request for security vulnerabilities and code quality issues. I'll provide an update when I'm done |
|
✅ I finished the code review, and didn't find any security or code quality issues. |
1 similar comment
|
✅ I finished the code review, and didn't find any security or code quality issues. |
AbdulR3hman
force-pushed
the
feature/docdb-managed-connector-rebased
branch
2 times, most recently
from
b502ef3 to
6fb20bc
Compare
| assertEquals("6", finalResponse.getNextToken()); | ||
|
|
||
| // Verify empty page after all data is consumed | ||
| ListTablesRequest emptyRequest = new ListTablesRequest(IDENTITY, QUERY_ID, DEFAULT_CATALOG, DEFAULT_SCHEMA, |
There was a problem hiding this comment.
this part of the test expects that the paging happens infinitely; and that the server pagination is stateful; which it isn't. by setting the next token to null; we are expecting to restart from the start which what happens here.
There was a problem hiding this comment.
therefore this part of the test isn't correct hence removed.
AbdulR3hman
force-pushed
the
feature/docdb-managed-connector-rebased
branch
from
6fb20bc to
4167417
Compare
AbdulR3hman
force-pushed
the
feature/docdb-managed-connector-rebased
branch
from
4167417 to
3d53fc7
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #3229 +/- ##
============================================
+ Coverage 68.33% 70.35% +2.02%
- Complexity 5021 5406 +385
============================================
Files 639 645 +6
Lines 24197 24884 +687
Branches 2992 3114 +122
============================================
+ Hits 16534 17508 +974
+ Misses 6227 5922 -305
- Partials 1436 1454 +18 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| * @return Fully constructed MongoDB connection string in format: mongodb://username:password@host:port/?jdbcParams | ||
| * @throws RuntimeException if JSON credential parsing fails or required parameters are missing | ||
| */ | ||
| private String getConfigOptionsFromFederatedIdentity(Map<String, String> configOptions) |
There was a problem hiding this comment.
Is there a reason to use this function instead of DocDbEnvironmentProperties? That should already contain the connection string, secrets, etc.
2 participants
Enabling DocDB Substrait implementation.
This is a refactored and rebased (for easier review) version of PR-3016