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

Add Configurable options for Req.Request in HttpClient #48

Closed
shijithkjayan opened this issue Aug 14, 2024 · 7 comments · Fixed by #51
Closed

Add Configurable options for Req.Request in HttpClient #48

shijithkjayan opened this issue Aug 14, 2024 · 7 comments · Fixed by #51

Comments

@shijithkjayan
Copy link
Contributor

shijithkjayan commented Aug 14, 2024

Description:

Hi @jaeyson,

I would like to request an enhancement to the ExTypesense.HttpClient module to allow developers to configure the options for Req.Request through the application configuration. This would provide greater flexibility and allow developers to tailor the HTTP client to their specific needs.

For example, it would be beneficial to configure the options like so:

config :ex_typesense,
  api_key: "XXXXXX",
  ...
  options: [finch: MyApp.Finch]

This change would enable developers to leverage the various options available in Req.Request, such as:

  • :finch - the name of the Finch pool.
  • :connect_options - dynamically starts (or re-uses already started) Finch pool with the given connection options.
  • :inet6 - if set to true, uses IPv6.
  • :pool_timeout - pool checkout timeout in milliseconds.
  • :receive_timeout - socket receive timeout in milliseconds.
  • :unix_socket - if set, connect through the given UNIX domain socket.
  • :finch_private - a map or keyword list of private metadata to add to the Finch request.
  • :finch_request - a function that executes the Finch request.

For more details on the available options, please refer to the Req documentation.

Reason for Request:

The primary reason for this request is to provide developers with the ability to configure the HttpClient to meet their specific requirements. By exposing the options for Req.Request as a configurable value, developers can easily adjust the behavior of the HTTP client without modifying the library code.

Use Case:

One specific use case for this enhancement is the need to use a different Finch adapter other than the default one started by Req. For instance, in a scenario where an application has multiple Finch pools configured for different services, a developer might want to specify a particular Finch pool for the HttpClient to use. This can be achieved by configuring the options as follows:

config :ex_typesense,
  api_key: "XXXXXX",
  ...
  options: [finch: MyApp.CustomFinch]

In this example, MyApp.CustomFinch is a custom Finch pool that the developer has configured with specific connection options or other settings that differ from the default Finch pool. This flexibility allows the developer to optimize the HTTP client's performance and behavior according to the application's unique requirements.

Thank you for considering this enhancement. I believe it will greatly improve the flexibility and usability of the ExTypesense library.

@jaeyson
Copy link
Owner

jaeyson commented Aug 14, 2024

@shijithkjayan no worries, just make a pull request and we'll merge it.

@shijithkjayan
Copy link
Contributor Author

@jaeyson #51 - Here's the PR

One thing I wanted to add to the PR is testing the HttpClient.request/2 function to validate that the options are applied to the request. My lack of knowledge in Req has made it difficult for me to do.

Initially, I wanted to use Mox to do the testing, but I don't think that's how you want to test things here, which is why I haven't done that either.

@jaeyson
Copy link
Owner

jaeyson commented Sep 1, 2024

Awesome @shijithkjayan, thank you! I have few questions as I looked at the PR:

  1. Is the missing piece before merging this to main branch is testing HttpClient.request/2?
  2. Aside from that mentioned above, is there anything else that's missing before merging?

@shijithkjayan
Copy link
Contributor Author

Hi @jaeyson, yes the test is the only missing piece. I have verified that the changes haven't broken any existing functionalities. But the lack of test coverage for the changes I added is what that worries me

@jaeyson
Copy link
Owner

jaeyson commented Sep 4, 2024

Got it @shijithkjayan, will do.

@jaeyson jaeyson linked a pull request Sep 4, 2024 that will close this issue
@jaeyson jaeyson closed this as completed Sep 4, 2024
@shijithkjayan
Copy link
Contributor Author

@jaeyson Can you please create a new release? Or do you want me to submit another PR with the updated changelog?

@jaeyson
Copy link
Owner

jaeyson commented Sep 10, 2024

@shijithkjayan thanks for the nudge, I will do the release 👌🏾. I'll add the test for request function on the next release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants