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

Cloud Spanner - R2DBC - Spring data sample #214

Merged
merged 9 commits into from
Feb 12, 2020
Merged

Conversation

elefeint
Copy link
Contributor

@elefeint elefeint commented Feb 11, 2020

This sample creates a table called BOOK on application startup, and deletes it prior to application shutdown.

It provides three endpoints showing Spring Data R2DBC manipulation of Cloud Spanner data:

  1. /list returns all books in the BOOKS table using the autoconfigured DatabaseClient.
  2. /add inserts a book using the semantic insert in the autoconfigured DatabaseClient.
  3. /search/{id} finds a specific book by ID using Spring Data R2DBC repository.

I owe an integration test, which I'd like to add in a follow-up PR.

@elefeint elefeint requested a review from dzou February 11, 2020 20:28
@dzou
Copy link
Contributor

dzou commented Feb 11, 2020

Very nicely done; just 1 comment about the index.html thing.

Copy link
Member

@meltsufin meltsufin left a comment

Choose a reason for hiding this comment

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

Looks great! Just some polishing nits.

elefeint and others added 2 commits February 12, 2020 13:08
Co-Authored-By: Mike Eltsufin <meltsufin@google.com>
<dependency>
<groupId>com.google.cloud</groupId>
<artifactId>cloud-spanner-r2dbc</artifactId>
<version>0.1.0</version>
Copy link
Contributor

Choose a reason for hiding this comment

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

Should these version numbers just be <version>${project.version}</version> ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, yes. Definitely should not be hardcoded.
I made spring-boot-parent the parent, which is messing with project.version.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Take a look -- I've added a hardcoded project version instead. This is necessary because the parent is a spring boot starter. I don't know; I might just get rid of the starter parent in a follow-up and add the spring boot maven plugin and spring artifact versions to the samples parent.

<dependency>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-starter-webflux</artifactId>
<version>2.2.4.RELEASE</version>
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this version number necessary? If so, should be it be the same as the parent starter which is <version>2.2.3.RELEASE</version> ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch; it should actually just derive its version from the spring boot parent.

dzou
dzou previously approved these changes Feb 12, 2020
@elefeint elefeint merged commit 568f628 into master Feb 12, 2020
@elefeint elefeint deleted the spring-data-sample branch February 12, 2020 22:43
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.

None yet

3 participants