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

Add tool library for updating Spanner schema using Liquibase. #109

Merged
merged 1 commit into from
May 11, 2022

Conversation

SanjayVas
Copy link
Member

@SanjayVas SanjayVas commented May 2, 2022

@SanjayVas SanjayVas force-pushed the sanjayvas-update-schema branch 2 times, most recently from 41e0135 to ded8a3d Compare May 3, 2022 00:41
@SanjayVas SanjayVas force-pushed the sanjayvas-liquibase branch from fc201a0 to c814c4f Compare May 3, 2022 17:24
@SanjayVas SanjayVas force-pushed the sanjayvas-update-schema branch 5 times, most recently from 518b338 to 90afdf5 Compare May 6, 2022 01:09
@SanjayVas SanjayVas force-pushed the sanjayvas-liquibase branch from c814c4f to 84d0b97 Compare May 6, 2022 01:12
@SanjayVas SanjayVas force-pushed the sanjayvas-update-schema branch 2 times, most recently from 05c7d84 to d579bd6 Compare May 6, 2022 17:56
@SanjayVas SanjayVas requested review from stevenwarejones and efoxepstein and removed request for stevenwarejones May 6, 2022 17:57
@SanjayVas SanjayVas marked this pull request as ready for review May 6, 2022 17:57
@SanjayVas SanjayVas force-pushed the sanjayvas-update-schema branch from d579bd6 to f4eef40 Compare May 6, 2022 18:24
@SanjayVas SanjayVas force-pushed the sanjayvas-liquibase branch from 764dd51 to cc6bd39 Compare May 6, 2022 18:24
@SanjayVas SanjayVas force-pushed the sanjayvas-update-schema branch from f4eef40 to 5c3d302 Compare May 6, 2022 18:24
@SanjayVas SanjayVas force-pushed the sanjayvas-liquibase branch from cc6bd39 to 6889c09 Compare May 6, 2022 19:55
@SanjayVas SanjayVas force-pushed the sanjayvas-update-schema branch from 5c3d302 to 310ae83 Compare May 6, 2022 19:55
Copy link
Contributor

@efoxepstein efoxepstein left a comment

Choose a reason for hiding this comment

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

Reviewed 5 of 5 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @SanjayVas and @stevenwarejones)


src/main/kotlin/org/wfanet/measurement/gcloud/spanner/tools/UpdateSchema.kt line 47 at r1 (raw file):

    val connectionString =
      if (flags.emulatorHost == null) {
        "jdbc:cloudspanner:/projects/${flags.projectName}/instances/${flags.instanceName}/" +

Maybe val databasePath = "projects/.../instances/.../databases/..." so you can reuse it between the two branches here (and also make it more compact).

That could even be a property on SpannerFlags.

@SanjayVas SanjayVas force-pushed the sanjayvas-liquibase branch from 6889c09 to db6afc1 Compare May 9, 2022 19:23
@SanjayVas SanjayVas force-pushed the sanjayvas-update-schema branch from 310ae83 to 7efaf64 Compare May 9, 2022 19:23
Copy link
Member Author

@SanjayVas SanjayVas left a comment

Choose a reason for hiding this comment

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

Reviewable status: 3 of 5 files reviewed, 1 unresolved discussion (waiting on @efoxepstein and @stevenwarejones)


src/main/kotlin/org/wfanet/measurement/gcloud/spanner/tools/UpdateSchema.kt line 47 at r1 (raw file):

Previously, efoxepstein (Eli Fox-Epstein) wrote…

Maybe val databasePath = "projects/.../instances/.../databases/..." so you can reuse it between the two branches here (and also make it more compact).

That could even be a property on SpannerFlags.

Done, and moved to SpannerFlags.

Copy link
Contributor

@stevenwarejones stevenwarejones left a comment

Choose a reason for hiding this comment

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

Reviewed 3 of 5 files at r1, 2 of 2 files at r2, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @efoxepstein)

@SanjayVas SanjayVas force-pushed the sanjayvas-liquibase branch from db6afc1 to 828df35 Compare May 9, 2022 23:10
@SanjayVas SanjayVas force-pushed the sanjayvas-update-schema branch from 7efaf64 to 00d90ea Compare May 9, 2022 23:10
@SanjayVas SanjayVas force-pushed the sanjayvas-liquibase branch from 828df35 to fe077e3 Compare May 10, 2022 19:49
@SanjayVas SanjayVas force-pushed the sanjayvas-update-schema branch 2 times, most recently from 0d27c21 to dfaff9a Compare May 10, 2022 19:51
Base automatically changed from sanjayvas-liquibase to main May 10, 2022 19:54
@SanjayVas SanjayVas force-pushed the sanjayvas-update-schema branch from dfaff9a to 696bbac Compare May 10, 2022 19:55
Copy link
Contributor

@stevenwarejones stevenwarejones left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r3, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @efoxepstein)

Copy link
Contributor

@efoxepstein efoxepstein left a comment

Choose a reason for hiding this comment

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

Reviewed 2 of 2 files at r2, 1 of 1 files at r3, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @SanjayVas)

@SanjayVas SanjayVas merged commit c85914c into main May 11, 2022
@SanjayVas SanjayVas deleted the sanjayvas-update-schema branch May 11, 2022 17:58
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.

3 participants