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

driver that can connect to both versions 1 and 2 #293

Merged
merged 6 commits into from
Nov 27, 2023
Merged

Conversation

alexradzin
Copy link
Collaborator

No description provided.

@alexradzin
Copy link
Collaborator Author

Document can be found here.

@alexradzin alexradzin marked this pull request as ready for review November 5, 2023 14:32
@alexradzin alexradzin requested a review from a team as a code owner November 5, 2023 14:32
Comment on lines +32 to +47
run-v1:
description: 'Run tests against Firebolt DB v1'
required: true
default: true
type: choice
options:
- 'true'
- 'false'
run-v2:
description: 'Run tests against Firebolt DB v2'
required: true
default: true
type: choice
options:
- 'true'
- 'false'
Copy link
Contributor

Choose a reason for hiding this comment

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

You can add workflow_dispatch section to V1 and V2 workflows to make them individually callable alongside the shared workflow. This way you don't need to have those input parameters.

Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

89.7% 89.7% Coverage
0.0% 0.0% Duplication

warning The version of Java (11.0.21) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17.
Read more here

.compress(false)
.accountId(accountId)
.host(UrlUtil.createUrl(systemEngineEndpoint).getHost())
.build();
Copy link
Contributor

Choose a reason for hiding this comment

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

Here, we don't set the database. Do we set it elsewhere?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure. Here we update initial loginPorperties where all initially provided properties are updated (including DB if it has been supplied). The only reason of this code is to override some properties, e.g. systemEngine=true (because this is system engine), corepress=false (because system engine cannot compress data) etc.

@alexradzin alexradzin changed the base branch from feature/FIR-21173-new-identity-support to master November 27, 2023 07:28
@alexradzin alexradzin merged commit 7bceb3e into master Nov 27, 2023
4 checks passed
@alexradzin alexradzin deleted the both-identities3 branch November 27, 2023 07:33
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