fix: add default org caching [IDE-1566] #518
Conversation
So we don't need to make API calls every time someone sets the org to "", e.g. on a cloned configuration.
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
| ORGANIZATION_SLUG string = "internal_org_slug" // ORGANIZATION_SLUG (string) returns the slug of the organization and correlates to the ORGANIZATION ID. | ||
| ORGANIZATION string = "org" // ORGANIZATION (string) sets/returns the Organization ID | ||
| ORGANIZATION_SLUG string = "internal_org_slug" // ORGANIZATION_SLUG (string) returns the slug of the organization and correlates to the ORGANIZATION ID. | ||
| DEFAULT_ORGANIZATION string = "default_org" // DEFAULT_ORGANIZATION (string) returns the default Organization ID, as specified for the user in the web UI. |
There was a problem hiding this comment.
Question: Is it really globally relevant to access this value? Otherwise if this is required for some sub functionality, I would recommend to register it close to the functionality. The reason I'm asking is that I'm worried users get confused between ORGANIZATION and DEFAULT_ORGANIZATION. So far DEFAULT_ORGANIZATION was not required by any extension or other GAF consumer.
There was a problem hiding this comment.
We would also want it in LS so we can replace this awful code I wrote while under a time crunch: https://github.com/snyk/snyk-ls/blob/main/domain/ide/command/folder_handler.go#L166
I realised we only need to compare the org Id and now slug, so if I can just query the default org and compare OR have a function in GAF I can call to check if an org Id is the default org Id, then that would be great.
There was a problem hiding this comment.
have a function in GAF I can call to check if an org Id is the default org Id
This sounds like great a small impact byproduct of the logic that is already available. So if we can reduce the amount of code we add and even reduce the possibility for incorrect usage, I would say it is a win for everyone.
| callback := func(_ configuration.Configuration, existingValue interface{}) (interface{}, error) { | ||
| // TODO - This function uses the outer (global) config and network access, so will not respect values set in the closures' (potentially cloned) configs. | ||
| existingString, ok := existingValue.(string) | ||
| if existingValue != nil && ok && len(existingString) > 0 { | ||
| return existingString, nil | ||
| } |
There was a problem hiding this comment.
curious when do we invalidate ?
I think we should invalidate cache on token change and api endpoint
So we don't need to make API calls every time someone sets the org to
"", e.g. on a cloned configuration.Required for snyk/snyk-ls#1115