Skip to content

Conversation

tinnou
Copy link

@tinnou tinnou commented Aug 14, 2019

Description

I use java-dataloader in coordination of graphql-java. I'm running into a use case where I need to lazily construct and register data loaders only if they don't exist already in the registry.

Currently, there is no exposed way to perform this action atomically through the DataLoaderRegistry since it rightfully encapsulates its concurrent hash map.

This PR exposes this capability.

Testing

unit tests covering the new method.
Note: Any thoughts about adding Spock to the mix later?

@tinnou
Copy link
Author

tinnou commented Aug 14, 2019

Looks like the CI build fails when trying to install the OracleJDK8.

From this link, a possible workaround is to add dist: trusty to the travis.yml file or to use openjdk8
https://travis-ci.community/t/error-installing-oraclejdk8-expected-feature-release-number-in-range-of-9-to-14-but-got-8/3766/6

@bbakerman
Copy link
Member

Note: Any thoughts about adding Spock to the mix later?

No objection at all. I love groovy testing via Spock. I am a convert.

This was a port of an existing project and hence Junit was used as it was previously used.

I would be happy for a future with new tests being written in Spock / Groovy

@bbakerman
Copy link
Member

Looks like the CI build fails when trying to install the OracleJDK8.
From this link, a possible workaround is to add dist: trusty to the travis.yml file or to use openjdk8

Would you be willing to put that config into this PR and try to get it working. That would be really helpful?

@bbakerman bbakerman self-requested a review August 17, 2019 03:18
Copy link
Member

@bbakerman bbakerman left a comment

Choose a reason for hiding this comment

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

Thanks for this improvement

@tinnou
Copy link
Author

tinnou commented Aug 22, 2019

Looks like the CI build fails when trying to install the OracleJDK8.
From this link, a possible workaround is to add dist: trusty to the travis.yml file or to use openjdk8

Would you be willing to put that config into this PR and try to get it working. That would be really helpful?

Oh yea absolutely, I wasn't sure what approach you wanted to go with. We can either add this dist: trusty (it looks like it did the trick) or we could switch to using OpenJDK as well. It might be a bigger endeavor so I opted for the former.

@bbakerman bbakerman merged commit 27a5b6b into graphql-java:master Aug 24, 2019
@bbakerman
Copy link
Member

@tinnou - 2.2.3 has been released with these changes

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.

2 participants