-
-
Notifications
You must be signed in to change notification settings - Fork 43
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 external engine analysis post endpoints #81
base: master
Are you sure you want to change the base?
Add external engine analysis post endpoints #81
Conversation
Could |
Hi @fitztrev, I am not sure if I understand what you mean. |
I mean when someone goes to use berserk in a script, they are currently able to specify a custom base_url instead of pointing to production See example here that points to |
@fitztrev Yes, should be no problem. That custom base url would need to be passed as an argument (custom_base_url) when calling one of the new endpoints methods (analysis, aquire_request and answer_request). |
I think it makes more sense to do it the same as with the tablebase or opening explorer where the custom URL can be set on the https://github.com/lichess-org/berserk/blob/master/berserk/clients/__init__.py#L91 https://github.com/lichess-org/berserk/blob/master/berserk/clients/tablebase.py Unfortunately, some external engine requests have to go against the default Licehss URL but I think it also still makes sense to use a similar pattern with a separate client that has the correct base URL set instead of passing it on every call. The ExternalEngine client could create a separate sub-client in its constructor with the external engine URL as its base URL. Or ExternalEngine could simply not subclass BaseClient and instead have two clients as attributes. I think either option is fine. Then we don't need custom URL params on any of the methods. And the default external engine URL can also be extracted to a constant at the top of the external engine file. |
@benediktwerner Thank you for the explanation, that makes sense and is indeed cleaner. |
Checklist when adding a new endpoint
Building on @Anupyas PR I moved the analysis endpoints into the external_engine client. I handled the different base URL with a new parameter in the Requestor methods post and request which accepts the different base url.README.md
client.users.get_user()
, Correct:client.users.get()
berserk/types/
, exampleCHANGELOG.md
in theTo be released
section (to be created if necessary)