-
Notifications
You must be signed in to change notification settings - Fork 14
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
feat(dart): add algolia_chopper_requester package #3291
base: main
Are you sure you want to change the base?
feat(dart): add algolia_chopper_requester package #3291
Conversation
Since I've added a completely new package here there were a few defaults I chose, which should probably be modified in this PR:
Both of these should be reviewed and potentially changed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
awesome, thanks a lot for the contribution!
Some comments about standard package structure across the API clients, for the dart code itself I think @aallam will know better
clients/algoliasearch-client-dart/packages/chopper_requester/LICENSE
Outdated
Show resolved
Hide resolved
clients/algoliasearch-client-dart/packages/chopper_requester/CHANGELOG.md
Outdated
Show resolved
Hide resolved
clients/algoliasearch-client-dart/packages/chopper_requester/README.md
Outdated
Show resolved
Hide resolved
clients/algoliasearch-client-dart/packages/chopper_requester/analysis_options.yaml
Outdated
Show resolved
Hide resolved
Ah nice I was actually commenting on that, do you want to do it with the pointers I've gave or should I take over this part? |
@shortcuts I think I've done all the tasks. I'm just not sure about d812167 😅 I synced version.dart with the Dart package version manually. Not sure if I should delete it since it's a dependency of chopper_requester.dart. |
The only annoying bit left to fix is the fact that the user has to provide their final SearchClient _client = SearchClient(
appId: appId, // <-- here
apiKey: apiKey, // <-- here
options: ClientOptions(
requester: ChopperRequester(
appId: appId, // <-- and here
apiKey: apiKey, // <-- and here
)
),
); If only there were a way for these credentials to be passed down from the CC/ @shortcuts @aallam |
…ge' into feat/add-chopper-requester-package
Indeed, this could be improved; however, I think this should be done in a separate PR since it's not specific to this package |
Agreed. |
Almost there to make CI work on forks! We will try to get that sorted out soon, thanks a lot for your contribution :D |
# Conflicts: # generators/src/main/java/com/algolia/codegen/AlgoliaDartGenerator.java
…ester-package # Conflicts: # generators/src/main/java/com/algolia/codegen/AlgoliaDartGenerator.java
# Conflicts: # generators/src/main/java/com/algolia/codegen/AlgoliaDartGenerator.java
🧭 What and Why
In this PR I have created a custom
Requester
based on Chopper.Why? Dio is great, but if my Flutter app uses http I now have to have 2 different clients. 🙈 On top of that Chopper gives the user the option to provide their own
Client
.Basic Usage
Advanced Usage
To set the connect timeout one has to do that directly on the
Client
, i.e.Custom Interceptors
For interceptors please see the Chopper documentation.
Custom Clients
Via the
client
option users can use platform specific HTTP clients such:NOTE: Custom HTTP clients must be manully disposed of, i.e.
Parsing JSON in the background using Isolates
Parsing JSON in the background is a good idea if you don't want to block the main thread.
Please see the Chopper documentation on Decoding JSON using Isolate worker pools.
Changes included:
AlgoliaAgent
fromalgolia_client_core
packagealgolia_chopper_requester
Full disclosure: I'm one of the maintainers of Chopper.
Supersedes algolia/algoliasearch-client-dart#14