#12. Use testcontainers. Use org.foundationdb artifacts and latest fdb image.

JanusGraph / janusgraph-foundationdb

FoundationDB storage adapter for JanusGraph

Other

53 stars 18 forks source link

#12. Use testcontainers. Use org.foundationdb artifacts and latest fdb image. #13

Closed jackson-chris closed 4 years ago

jackson-chris commented 4 years ago

All the test classes were passing except FoundationDBPartitionGraphTest. This test just seems to hang, it's unclear to me if this behavior is related to my change or is pre-existing or environmental. I have Ignored it for this PR. If I get feedback from others that this test is currently passing I will troubleshoot further, otherwise my intent is to leave it as Ignored and address it as a separate issue.

janusgraph-bot commented 4 years ago

Committer of one or more commits is not listed as a CLA signer, either individual or as a member of an organization.

linux-foundation-easycla[bot] commented 4 years ago

The committers are authorized under a signed CLA.

:white_check_mark: Christopher Jackson (f3a1478ff9933238441ad380a841fc18e967a89a)

rngcntr commented 4 years ago

I can reproduce your failing test case which also hangs for me. From what I see, the following line in getMultiRange() does not finish: https://github.com/JanusGraph/janusgraph-foundationdb/blob/f3a1478ff9933238441ad380a841fc18e967a89a/src/main/java/com/experoinc/janusgraph/diskstorage/foundationdb/FoundationDBTx.java#L226

What I don't know is if that is due to a logical error in the adapter or some sort of misconfiguration elsewhere.

mbrukman commented 4 years ago

@farodin91 wrote:

@mbrukman My plan is to merge this first than work on travis ci which is afterward much easier.

SGTM!

farodin91 commented 4 years ago

@mbrukman Otherwise, i would merge it and start working on the travis ci integration.

mbrukman commented 4 years ago

@farodin91 — looks like @blindenvy just squashed the commits and re-pushed, but FYI, GitHub has an option for "squash and merge" in the "Merge pull request" dropdown; I'm guessing you get to edit the combined squashed commit message. Feel free to use it in the future if you wish to speed up the process and let me know how it goes!

farodin91 commented 4 years ago

@mbrukman I will let you know.

Our normal review process requires two approvals or lazy consensus of 7 days. Therefore, i asked for an review.

jackson-chris commented 4 years ago

@mbrukman please re-review when you can.

jackson-chris commented 4 years ago

If no objections I will leave it as is and we can use the 'squash and merge' functionality.