feat: Fir 12126 make engine parameter optional #152
Conversation
stepansergeevitch
changed the title
|
Lgtm, just wondering if we can use |
Oh, that actually seems to be much better, didn't see that endpoint. Will use this one |
| assert database is not None | ||
|
|
||
| if engine_name: | ||
| if not engine_name and not engine_url: |
There was a problem hiding this comment.
Change the documentation on L153 to reflect the changes
| HTTPStatusError, | ||
| KeyError, | ||
| ) as e: | ||
| raise InterfaceError(f"Unable to retrieve default engine endpoint: {e}.") |
There was a problem hiding this comment.
What if the real error is wrong credentials or expired/incorrect token?
There was a problem hiding this comment.
Sure, we could also catch AuthenticationError
There was a problem hiding this comment.
On the other side, I think AuthenticationError is pretty representative itself, so I think we should just let it propagate further
|
Kudos, SonarCloud Quality Gate passed! |
4 participants
Allow users to skip engine parameter. Get default engine url in this case.
Added unit and integration tests