sdk/java: accept multiple URLs in Client #486

Merged
merged 4 commits into from Feb 8, 2017

Projects

None yet

4 participants

@jbowens
Member
jbowens commented Feb 7, 2017 edited

Allow the Java SDK Client to be configured with multiple cored or
signerd URLs for high availability in the absence of a load balancer.

@jbowens jbowens added the PTAL label Feb 7, 2017
@kr
Member
kr commented Feb 7, 2017

๐Ÿ‘€

+
+ // A request to the url at failedIndex just failed. Move to the next
+ // URL in the list.
+ int nextIndex = (failedIndex + 1) % this.urls.size();
@kr
kr Feb 7, 2017 Member

This looks like a data race on this.urls if addURL or setURL is called concurrently. Should the docs for those methods say they mustn't be called concurrently with any RPCs?

@jbowens
jbowens Feb 7, 2017 Member

Those methods are on the Builder whereas this one is on the Client. Once a Client is constructed, urls should be immutable.

@kr
kr approved these changes Feb 8, 2017 View changes
@kr
Member
kr commented Feb 8, 2017

LGTM

@jeffomatic
Member

๐Ÿ‘€

@jeffomatic

LGTM except for one comment.

+ * Adds a base URL for the client to use.
+ * @param url the URL of the Chain Core or HSM.
+ */
+ public Builder addURL(URL url) throws BadURLException {
@jeffomatic
jeffomatic Feb 8, 2017 Member

I don't think this method throws BadURLException.

@jbowens
Member
jbowens commented Feb 8, 2017

Tested it locally, and it worked okay, but I discovered #487.

jbowens added some commits Feb 7, 2017
@jbowens @chainbot jbowens sdk/java: accept multiple URLs
Allow the Java SDK Client to be configured with multiple cored or
signerd URLs for high-availability in the absence of a load balancer.
663c64c
@jbowens @chainbot jbowens jfmt f6c9cfe
@jbowens @chainbot jbowens BadURLException not thrown 016df15
@jbowens @chainbot jbowens builder constructor urls
02408fa
@chainbot chainbot merged commit 66f8518 into main Feb 8, 2017

3 checks passed

licence/cla Contributor License Agreement is signed.
Details
wercker/cored Wercker pipeline passed
Details
wercker/java Wercker pipeline passed
Details
@chainbot chainbot deleted the java-multiple-coreds branch Feb 8, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment