Skip to content

Conversation

daschl
Copy link
Contributor

@daschl daschl commented Nov 19, 2013

This adds a starter and autoconfigure support for spring-data-couchbase!

@daschl
Copy link
Contributor Author

daschl commented Nov 19, 2013

Ah, I see what's going on. Let's wait until @olivergierke fixes the pushing to the milestone repo and then I'll update this PR to use M2.. then the correct couchbase client is used also.

Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this thing be part of SD Couchbase?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

should it? I created it because jpa and mongo also do it like this in there.

Copy link
Member

Choose a reason for hiding this comment

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

Okay, maybe I am not too up to date with the way Boot handles this. The registrars are usually refered to from @Enable…Repositories annotation and there should be a similarly named type in the actual Spring Data project backing this annotation. Maybe @dsyer can elaborate on this one?

Copy link
Member

Choose a reason for hiding this comment

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

It's a tricky question actually. I like the fact that Spring Boot is a one-stop shop for opinions like this one, and Michael is right, there are already 2 such implementations in Spring Boot (for JPA and Mongodb). But on the other hand Spring Data Couchbase is a relatively new project if I understand correctly, and if is not GA by the time we want to release Boot, then this code could not be included there.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dsyer when do you plan to have a GA? spring data couchbase is expected to GA end of 2013. We are pretty much feature complete and only stabilising the codebase.

Copy link
Member

Choose a reason for hiding this comment

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

Same here really. If you can get a GA release out then we can merge this whenever that happens.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So there is no chance we can get people to try it out and give feedback?

Copy link
Member

Choose a reason for hiding this comment

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

I said it was tricky. How about if we add the milestone dependency but pull out this feature from a release if the GA dates fall in the wrong order?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sounds good to me!

I'll update this one as soon as the m2 is synced in the right repo, I dont want to add another repository definition (libs-milestone) to spring-boot. what do you think?

@daschl
Copy link
Contributor Author

daschl commented Mar 13, 2014

@dsyer hey, we'll do a 1.0 GA today of the couchbase module. Is it still possible to get - once updated - into 1.0 of boot? Or is it already closed for new features and we'll wait until the next minor?

@dsyer
Copy link
Member

dsyer commented Mar 13, 2014

We'd prefer to wait for 1.0.1 if that's OK with you. Shouldn't be long coming.

@daschl
Copy link
Contributor Author

daschl commented Mar 13, 2014

@dsyer no worries, perfectly fine for me. I'll fix up the PR as soon as 1.0 GA is out and then you can choose when to merge. I'll ping you once done. cheers!

@dsyer
Copy link
Member

dsyer commented Mar 13, 2014

If we are going to have an RC5 I'll try and remember to merge this. Otherwise we'll wait for 1.0.1.

@daschl
Copy link
Contributor Author

daschl commented Mar 13, 2014

@dsyer cool. one more question.. is the whole registrar thing gone? I checked the current impls and they more or less just add a pom.xml with dependencies..

Or do I need to implement the registrar and all of that in the spring-data package itself?

@daschl
Copy link
Contributor Author

daschl commented Mar 13, 2014

ah okay its directly in the autoconfigure sources, gotcha.

@philwebb philwebb added this to the 1.0.1.RELEASE milestone Mar 13, 2014
@philwebb philwebb modified the milestones: 1.1.0, 1.0.1 Apr 3, 2014
@dsyer dsyer closed this in 78ce06c Apr 25, 2014
@dsyer
Copy link
Member

dsyer commented Apr 28, 2014

@daschl I'm going to back these changes out while we sort a few issues out (hopefully nothing major). If you want to send fixes probably the best is to send a PR based off commit 59a1b39. Here are some questions, and comments:

  1. It appears that CouchbaseClient tries to connect to a server on startup which causes an app to fail if the server isn't there. A lazy connection attempt would be better and more in keeping with other Spring Data projects.

  2. There is no explicit support for TLS/HTTPS in the autoconfig you proposed. Surely that would be necessary?

  3. I noticed an AbstractCouchbaseConfiguration in the Spring Data project but you didn't use it in the Spring Boot support. Wouldn't it be useful?

  4. It would be easy to support a List<String> for the hosts (and that seems to be conventional in Couch client)?

  5. I already separated the CouchbaseClient configuration from the repositories configuration (like in the MongoDB case). Is that OK?

@dsyer dsyer reopened this Apr 28, 2014
@daschl
Copy link
Contributor Author

daschl commented Apr 30, 2014

@dsyer I'm out this week but will sort it out next week if that is okay

@dsyer
Copy link
Member

dsyer commented Apr 30, 2014

No problem.

@philwebb philwebb modified the milestones: 1.1.0.M2, 1.1.0.M1 May 7, 2014
@daschl
Copy link
Contributor Author

daschl commented May 13, 2014

Okay, quick update here:

  1. Is hard to fix with the current SDK, but this will change with the new one that is currently under development. if not absolutely needed, let's keep it as is since it will vanish this year anyway I hope.

  2. the current SDK does not support SSL, but the new one will with a flag that you can enable. So we need to defer this too to the rewrite.

  3. definitely!

  4. yes!

  5. sounds good.

So I'll do PRs for 3 and 4 then.

@philwebb
Copy link
Member

We're getting close to RC, should I push this one back to 1.2 or do you think we can still get there for 1.1?

@philwebb philwebb modified the milestones: 1.1.0.RC1, 1.1.0.M2 May 26, 2014
@daschl
Copy link
Contributor Author

daschl commented May 27, 2014

@philwebb hey, let's push back on this I'm running out of time and I'll do this as part of a major overhaul during summer (new spring-data-couchbase for new SDK).

@daschl
Copy link
Contributor Author

daschl commented May 27, 2014

Also, people can plugin in sdc very easily anyway so its not a huge issue for them

@philwebb
Copy link
Member

Sounds good. I'll move it to 1.2. Cheers.

@philwebb philwebb modified the milestones: 1.2.0, 1.1.0.RC1 May 27, 2014
mdeinum pushed a commit to mdeinum/spring-boot that referenced this pull request Jun 6, 2014
@trisberg trisberg modified the milestones: 1.1.4, 1.2.0 Jul 3, 2014
@philwebb philwebb removed this from the 1.1.4 milestone Jul 3, 2014
@ldoguin
Copy link

ldoguin commented Oct 28, 2014

@dsyer Would the lazy-init option fix the CouchbaseClient connection issue?

@wilkinsona
Copy link
Member

We got there in the end. Auto-configuration support for Spring Data Couchbase was added in 76f1ca4 which was released in 1.4.0.M1.

@wilkinsona wilkinsona closed this Apr 6, 2016
@wilkinsona wilkinsona removed type: enhancement A general enhancement status: waiting-for-feedback We need additional information before we can continue labels Apr 6, 2016
philwebb pushed a commit to philwebb/spring-boot that referenced this pull request May 31, 2024
philwebb pushed a commit to philwebb/spring-boot that referenced this pull request May 31, 2024
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.

7 participants