-
Notifications
You must be signed in to change notification settings - Fork 328
Description
#627 resolved the "issue" #617 with a solution which I believe is unacceptable. KeycloakAdmin.get_group_by_path no longer throws an error when the group was not found, returning the error message from the method instead. This behavior is unexpected and undocumented.
Calling
keycloak_admin.get_group_by_path('/nonexistent_group')returns
{'error': 'Group path does not exist'}Especially with Python being a dynamically typed language, the unexpected return value can propagate to other code and make tracing the error difficult, if the method user does not add a check for the response being containing an error field.
The same complaint was voiced by others in the original issue:
- Keycloak admin get_group_by_path raises exception by not found #617 (comment)
- Keycloak admin get_group_by_path raises exception by not found #617 (comment)
I believe the change from #627 should be reverted. A custom "Not found" exception can optionally be added, but the best solution would be for the method's not found error handling to match those of other methods which can fail with a 404 status code.
Please note that most fixes might affect existing code, but should not make any existing code incorrect - the same is not true for the "fix" introduced in #627.