remove configuration responsible for transitioning to logical synchronizer deployment#5708
Conversation
[backport] ReminderPlease consider backporting to the following branches:
And your PR is currently against base branch: main. Note: Any PR comment containing [backport] will be considered for auto-backporting upon merge, |
…nizer deployment [static] Signed-off-by: Mateusz Błażejewski <mateusz.blazejewski@digitalasset.com>
mblaze-da
force-pushed
the
mblaze-da/lsu-intro-config-cleanup/5707
branch
from
ca25f89 to
fd946f3
Compare
| if [ "$#" -gt 1 ]; then | ||
| _error "Usage: $SCRIPTNAME enable_logical_synchronizer_deployment [migration_type (Default: active)]" | ||
| fi | ||
| _update_cluster_config 'true' "synchronizerMigration.$migration_type.enableLogicalSynchronizerDeploymentMode" |
There was a problem hiding this comment.
any reason not to just remove it in one go? (will require we update the splice bump to remove it's usage but might be easier)
works either way for me, though in this scenario we might wanna add a todo for it
There was a problem hiding this comment.
I'm planning to immediately follow up with a splice-bump, pr to internal and lastly a PR to splice that cleans those up.
There was a problem hiding this comment.
The main purpose for leaving those was that I wanted to do an lsu_test and conclude that I did not break anything. But the test failed on base deploy :/ I'm really confused why it did not fail in the same way when doing a cluster_test.
There was a problem hiding this comment.
does it make sense that this file is in common now? can we move it to the sv project?
| attachPvc: z.boolean().default(true), | ||
| lsuEnabled: z.boolean().default(false), | ||
| frozenMigrationId: z.number().optional(), | ||
| // legacy config, to be removed once all usage is removed |
There was a problem hiding this comment.
same point as above, either we purge it in one go or we add a todo item
| withAddedDependencies(opts, ledgerApiUserSecret ? [ledgerApiUserSecret] : []) | ||
| ) | ||
| : { chart: undefined, db: undefined }; | ||
| const { chart: participant, db: participantDb } = { chart: undefined, db: undefined }; |
There was a problem hiding this comment.
does this still make sense to even keep here?
There was a problem hiding this comment.
You're right, forgot to remove those.
[static] Signed-off-by: Mateusz Błażejewski <mateusz.blazejewski@digitalasset.com>
[static] Signed-off-by: Mateusz Błażejewski <mateusz.blazejewski@digitalasset.com>
[static] Signed-off-by: Mateusz Błażejewski <mateusz.blazejewski@digitalasset.com>
|
/cluster_test |
|
Deploy cluster test triggered for Commit d8a7239678f5aa3729815aa9f15eb71d1f3410c4 in , please contact a Contributor to approve it in CircleCI: https://app.circleci.com/pipelines/github/DACH-NY/canton-network-internal/67411 |
|
/lsu_test |
|
Deploy LSU pipeline triggered for Commit d8a7239678f5aa3729815aa9f15eb71d1f3410c4 in , please contact a Contributor to approve it in CircleCI: https://app.circleci.com/pipelines/github/DACH-NY/canton-network-internal/67604 |
[static] Signed-off-by: Mateusz Błażejewski <mateusz.blazejewski@digitalasset.com>
3 participants
part of #5707
cluster test: https://app.circleci.com/pipelines/github/DACH-NY/canton-network-internal/67412
lsu test: https://app.circleci.com/pipelines/github/DACH-NY/canton-network-internal/67605
Somehow, I managed to break the base deployment in an lsu_test but it still works in a cluster_test. AFAICT the issue in lsu_test was that the participant script responsible for creating the DB had some auth issues but I'm not sure. I did not touch the postgres layer so I'm wondering if it wasn't broken already before my change.