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

GORA-437 implement couchdb datastore #74

Merged
merged 4 commits into from
Aug 23, 2016
Merged

Conversation

cguzel
Copy link
Member

@cguzel cguzel commented Aug 13, 2016

This pull request is for GORA-437. I developed couchdb store for gora. I added javadocs and test codes. The tests required couchdb instance. But I wondered how to start couchdb server programmatically using java. So, I sent an email to couchDB userlist. They suggest that using docker. Then I use docker programmaticaly with https://github.com/testcontainers/testcontainers-java . All test cases passed with this approach. But this caused dependece to docker and jdk 1.8 . So this pull request doesn't merge to master.

We should look another solution. Maybe using mockito for testing or another idea?

All tests are passed on my local. I ignored 3 tests. You can see them in my test code.

@@ -690,6 +690,7 @@ else if (effectiveSchema.getType() == Type.MAP) {
}
// continue like a regular top-level union
case RECORD:
//TODO duplicated code @see org.apache.gora.util.IOUtils#serialize()
Copy link
Member

Choose a reason for hiding this comment

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

What do you suggest implementing here? Please open a ticket in Jira.

Copy link
Member Author

Choose a reason for hiding this comment

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

I have created a new issue : https://issues.apache.org/jira/browse/GORA-487

Copy link
Member Author

Choose a reason for hiding this comment

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

I have created new pull request for it as follow: #76

@lewismc
Copy link
Member

lewismc commented Aug 14, 2016

In general this is looking very good. Can you please go through and address my comments? Can you also ensure that for every new method that you've added you also add a Javadoc comment?
Finally, for the time being at least, can you

  1. Add package-info.java for each and every new module you've added.
  2. Create documentation for this module as per http://gora.apache.org/current/index.html#gora-modules

Thanks @cguzel great work :)

@lewismc
Copy link
Member

lewismc commented Aug 20, 2016

@cguzel can you please rebase against master?

@cguzel
Copy link
Member Author

cguzel commented Aug 20, 2016

@lewismc How to create documentation? I don't know how to edit this page : http://gora.apache.org/current/index.html#gora-modules

@lewismc
Copy link
Member

lewismc commented Aug 20, 2016

@cguzel
Copy link
Member Author

cguzel commented Aug 21, 2016

I added a patch for documentation https://issues.apache.org/jira/browse/GORA-489 @lewismc


final CouchDBStore store;

public CouchDBQuery() {
Copy link
Member

Choose a reason for hiding this comment

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

javadoc

private ObjectMapper instance;
private boolean writeDatesAsTimestamps = false;

public synchronized ObjectMapper createObjectMapper() {
Copy link
Member

Choose a reason for hiding this comment

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

javadoc

@cguzel
Copy link
Member Author

cguzel commented Aug 21, 2016

I have added maven profile in gora-couchdb pom.xml. So The tests skip by default. If you use java 1.8 and docker, you build and run tests using maven profile as "mvn clean install -P couchdb-with-test".

@cguzel
Copy link
Member Author

cguzel commented Aug 21, 2016

I have added new javadocs.

@asfgit asfgit merged commit 788807e into apache:master Aug 23, 2016
@bsideup
Copy link
Member

bsideup commented Feb 21, 2017

Hey@cguzel,

FYI TestContainers dependency has "compile" scope. You probably want to move it to "test" scope :)

@cguzel
Copy link
Member Author

cguzel commented Feb 21, 2017

Thanks @bsideup . I will check it.

@bsideup
Copy link
Member

bsideup commented Feb 21, 2017

@cguzel we're planning to add Apache Gora to TestContainers' "who is using?" list, don't you mind?
testcontainers/testcontainers-java#296

@cguzel
Copy link
Member Author

cguzel commented Feb 21, 2017

Hi, @bsideup . It's OK for me. But I am not PMC for GORA. So, This question should be answered by GORA PMC members.
@lewismc ?

@bsideup
Copy link
Member

bsideup commented Feb 21, 2017

@cguzel did you mean @lewismc ?

@cguzel
Copy link
Member Author

cguzel commented Feb 21, 2017

Sorry. Yes true. I corrected the mistake.

@lewismc
Copy link
Member

lewismc commented Feb 21, 2017

Absolutely @bsideup @cguzel thank you. Let's get a ticket filed to change scope to test as well

@bsideup
Copy link
Member

bsideup commented Feb 21, 2017

@lewismc may I ask you to comment or react with " 👍 " on testcontainers/testcontainers-java#296 ? :)

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.

4 participants