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

Gives HttpQuery subclasses ability to customize HTTP client #182

Merged
merged 3 commits into from
Sep 28, 2020
Merged

Gives HttpQuery subclasses ability to customize HTTP client #182

merged 3 commits into from
Sep 28, 2020

Conversation

bmomcilov
Copy link
Contributor

No description provided.

@barspi
Copy link
Contributor

barspi commented Sep 28, 2020

I'm not sure how you want to subclass HttpQuery
Maybe you want to subclass the client building process, but buildClient() already calls build() (does it finalize the building?)
I think a better idea would be to have a getClientBuilder() that returns a not-yet-built() client, so you can override that method, call super(), and then add your own build options.

@bmomcilov
Copy link
Contributor Author

I'm not sure how you want to subclass HttpQuery
Maybe you want to subclass the client building process, but buildClient() already calls build() (does it finalize the building?)
I think a better idea would be to have a getClientBuilder() that returns a not-yet-built() client, so you can override that method, call super(), and then add your own build options.

This way you can extend HttpQuery class, override buildClient(...) and completely build HTTP instance yourself which would be helpful for testing purposes (to actually return mock instance). As long as it confirms CloseableHttpAsyncClient abstract it is ok.

@barspi
Copy link
Contributor

barspi commented Sep 28, 2020

ahh, mocking is your business ;-)
In any case, maybe your needs, and what I explained above can co-exist if it's not too much trouble

@bmomcilov
Copy link
Contributor Author

Not at all, let me add that.

@ar ar merged commit bcdbe56 into jpos:master Sep 28, 2020
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.

3 participants