-
Notifications
You must be signed in to change notification settings - Fork 389
feat(calling): U2C catalog cache logic #4569
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
Conversation
|
This pull request is automatically being deployed by Amplify Hosting (learn more). |
Coread
left a comment
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.
There are a number of issues here:
- The preauth catalog is always assumed to be the same, regardless of the user. But this is not the case. You can get a different catalog due to proximity (e.g. VPN/no VPN) and entering a different email will also result in a different catalog
- After auth, the assumption that the same org means the same catalog is also not the case. Orgs can have users on different clusters and so get different catalogs
- This code is written in services v1, when we are moving soon to services v2.
- The code references local storage directly. In the SDK we use either bounded or unbounded storage, letting the consumers of the library choose then how to actually store the data.
Thanks for the review @Coread. This PR is raised with changes in order to check the feasibility of this solution so once we have the approval on approach we will replicate the changes on v2 catalog as well. Also was facing some issues with bounded storage to get the review started but plan is to use boundedStorage only. I will be making those changes in this PR itself Now for 1st and 2nd point, I will address those points. Could you please help me identify in what all cases the catalog changes or redirect me to where I can find this information. So I can consider all the cases here |
chrisadubois
left a comment
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.
I do not recall having changing the concept of how we handle u2c catalogs. We rely pretty heavily on the fact we will always get a fresh u2c catalog. In fact, as part of the u2cv2 migration, we are not handling the failsafe hostmap because we have no cache for the u2c catalog. This changes us to follow the behavior of what the native client does, and that has some drawbacks that impact the application.
Also, this seems like the wrong approach, opens a bigger can of worms than it resolves. Perhaps the root issue is that the API is slow? IMO this is high risk low reward
|
Please make this change a configuration initialization option. |
|
Closing this one, Will raise new PR with changes applied only for calling based on config passed |
COMPLETES #https://jira-eng-sjc12.cisco.com/jira/browse/CAI-6513
This pull request addresses
We are storing the U2C catalog for 24 hours to avoid fetching it if frequent browser refresh occur.
by making the following changes
Change Type
The following scenarios were tested
Refreshed browser and observed in the network that U2C request was not being fetched anymore if entry in cache is found.
The GAI Coding Policy And Copyright Annotation Best Practices
I certified that
Make sure to have followed the contributing guidelines before submitting.