-
Notifications
You must be signed in to change notification settings - Fork 124
refactoring: Standardize format of search params in engine configs #122
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
refactoring: Standardize format of search params in engine configs #122
Conversation
for more information, see https://pre-commit.ci
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.
Great start.
I realised that [{search_params: { parallel: 100, config: {...} } }]
would be better than [{search_params: { parallel: 100, params: {...} } }]
Wdyt? If you agree, please change accordingly.
LGTM. Please confirm that you've tested all the engines once and then we can merge this. |
@KShivendu I have tested all the engines once with |
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.
Great. I've tested the engines from my end too 🚀
this pr is breaking backwards compatibility with the previous versions, isn't it? |
We discussed and agreed that it's okay. |
Make search_params consistent across all the engines. The new format is:
"search_params": [ { "parallel": 1, "config": { ... } }, ... ],