Skip to content

feat: blue/green support#244

Open
JuanLeee wants to merge 1 commit intoaws:mainfrom
JuanLeee:feat/bg
Open

feat: blue/green support#244
JuanLeee wants to merge 1 commit intoaws:mainfrom
JuanLeee:feat/bg

Conversation

@JuanLeee
Copy link
Copy Markdown
Contributor

@JuanLeee JuanLeee commented Feb 26, 2026

Summary

Description

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@JuanLeee JuanLeee force-pushed the feat/bg branch 2 times, most recently from af690c3 to ecc5807 Compare February 27, 2026 18:08
karenc-bq
karenc-bq reviewed Mar 2, 2026
View reviewed changes
@kenrickyap
Copy link
Copy Markdown
Contributor

kenrickyap commented Mar 3, 2026

I think the csproj file is missing for BlueGreenConnection and BlueGreenConnection.Tests projects.

kenrickyap
kenrickyap reviewed Mar 5, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@kenrickyap kenrickyap left a comment

Choose a reason for hiding this comment

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

NOICE WORK :):) added some comments

karenc-bq
karenc-bq reviewed Mar 6, 2026
View reviewed changes
faviansamatha
faviansamatha reviewed Mar 6, 2026
View reviewed changes
faviansamatha
faviansamatha reviewed Mar 6, 2026
View reviewed changes
...pperDataProvider.Plugin.BlueGreenConnection/BlueGreenConnection/BlueGreenConnectionPlugin.cs Outdated
this.pluginService = pluginService;
this.props = props;
this.providerSupplier = providerSupplier;
this.bgdId = PropertyDefinition.BgdId.GetString(this.props);
Copy link
Copy Markdown
Contributor

@faviansamatha faviansamatha Mar 6, 2026

Choose a reason for hiding this comment

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

Should we normalize this like jdbc? Require it to be non null, trim, and also lowercase?

And curious, can users ever pass in null through the properties? This may blow up and we will get a NPE if we pass this in to routing.apply. Perhaps we should make bgId string instead of string? if possible

Copy link
Copy Markdown
Contributor Author

@JuanLeee JuanLeee Mar 17, 2026

Choose a reason for hiding this comment

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

I think it will use default value of property

faviansamatha
faviansamatha reviewed Mar 7, 2026
View reviewed changes
@JuanLeee JuanLeee force-pushed the feat/bg branch 6 times, most recently from 0473cd6 to 3df503d Compare March 18, 2026 18:51
karenc-bq
karenc-bq previously requested changes Mar 20, 2026
View reviewed changes
@karenc-bq karenc-bq self-requested a review March 20, 2026 08:52
@karenc-bq karenc-bq dismissed their stale review March 20, 2026 08:53

Misclick

@JuanLeee JuanLeee force-pushed the feat/bg branch 2 times, most recently from d7c156a to 0035588 Compare March 25, 2026 17:49
@JuanLeee JuanLeee force-pushed the feat/bg branch 2 times, most recently from 82f3958 to 74b80bd Compare March 26, 2026 22:43
karenc-bq
karenc-bq reviewed Mar 26, 2026
View reviewed changes
karenc-bq
karenc-bq reviewed Mar 26, 2026
View reviewed changes
@JuanLeee JuanLeee force-pushed the feat/bg branch 2 times, most recently from 9dcd290 to 0ff9f2e Compare March 26, 2026 23:37
karenc-bq
karenc-bq reviewed Mar 27, 2026
View reviewed changes
AwsWrapperDataProvider/Driver/Plugins/BlueGreenConnection/Routing/SuspendExecuteRouting.cs Outdated

while (bgStatus is { CurrentPhase: BlueGreenPhaseType.IN_PROGRESS })
{
await this.Delay(this.sleepTimeMs, bgStatus, this.BgdId, cts.Token);
Copy link
Copy Markdown
Contributor

@karenc-bq karenc-bq Mar 27, 2026

Choose a reason for hiding this comment

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

Looks like we are using cts.Token to interrupt this.Delay, curious if that will also interrupt this while loop while (bgStatus is { CurrentPhase: BlueGreenPhaseType.IN_PROGRESS }) or if you would still need stopwatch.ElapsedMilliseconds < timeoutMs

Copy link
Copy Markdown
Contributor Author

@JuanLeee JuanLeee Mar 27, 2026

Choose a reason for hiding this comment

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

It should interrupt the loop but I think putting in the time check would align better with the JDBC implementation.

karenc-bq
karenc-bq reviewed Mar 27, 2026
View reviewed changes
karenc-bq
karenc-bq reviewed Mar 27, 2026
View reviewed changes
@JuanLeee JuanLeee force-pushed the feat/bg branch 2 times, most recently from 09d7ee2 to 6a74abd Compare March 27, 2026 18:20
yanw-bq
yanw-bq reviewed Mar 30, 2026
View reviewed changes
karenc-bq
karenc-bq reviewed Mar 31, 2026
View reviewed changes
AwsWrapperDataProvider/Driver/Dialects/RdsMultiAzDbClusterPgDialect.cs

private static readonly string TopologyQuery =
$"SELECT id, endpoint, port FROM rds_tools.show_topology('aws_dotnet_driver-{DriverVersion}')";
$"SELECT id, endpoint, port FROM rds_tools.show_topology('aws_advanced_dotnet_data_provider_wrapper-{DriverVersion}')";
Copy link
Copy Markdown
Contributor

@karenc-bq karenc-bq Mar 31, 2026

Choose a reason for hiding this comment

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

could we grab the version dynamically here too

Copy link
Copy Markdown
Contributor Author

@JuanLeee JuanLeee Mar 31, 2026

Choose a reason for hiding this comment

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

Going to change in future PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@faviansamatha faviansamatha faviansamatha left review comments

@yanw-bq yanw-bq yanw-bq left review comments

@karenc-bq karenc-bq karenc-bq approved these changes

@kenrickyap kenrickyap kenrickyap approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@JuanLeee @kenrickyap @faviansamatha @karenc-bq @yanw-bq