Added retry to all scimma auth get requests. Removed verifying and au… - #95
Conversation
…to-regenerating the credential on failure.
There was a problem hiding this comment.
This generally looks good to me, with one note of a possible latency optimization.
I never really understood before what was causing the existing version to decide it needed to replace the Hopskotch credential for a user, particularly several times in parallel (presumably due to concurrent user API requests). Is that understood, or has the new version been tested under similar conditions? Maybe the removal of the redundant validity checks is what fixes/prevents this.
| if not hermes_group_name in [group['group'] for group in user_groups]: | ||
| add_user_to_group(username, hermes_group_name, hermes_api_token) | ||
| user_group_names.append(hermes_group_name) | ||
| # add User credential to groups if they are not already a part of them |
There was a problem hiding this comment.
It's a bit more complex, but you might want to do each phase of this process by building up a single multi-request, one multi request to put the user into all relevant groups, one to attach each relevant permission to the credential, etc. That way, for a user in many groups you incur O(1) network round-trips, rather than O(N).
It's not understood what was causing it, but by doing away with it altogether i hope we circumvent the problem. I think there is always the potential for transient issues in network communication with services, but I don't know if it was something more systemic then that. |
2 participants
…to-regenerating the credential on failure.
I realized that since all our calls to SCiMMA inherently check the credential anyway, and will fail if the credential is bad or doesn't have proper permissions, we gain little by verifying the credential stored in HERMES before each call, especially now that we expect the credential to stay the same forever. So I've removed all the code that would verify the credential and regenerate before each API call.
I've also enhanced the code when a user intentionally revokes their hop credential, so that it attempts to add them back to the same groups with the same permissions after deleting and generating the new credential.
The last part is just adding some more error handling and automatic retries for all GET requests to scimma admin.