-
Notifications
You must be signed in to change notification settings - Fork 14
fix: add default org caching [IDE-1566] #518
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -32,8 +32,9 @@ const ( | |
| // Org related configuration | ||
| // --------- | ||
|
|
||
| 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. | ||
| 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. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
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. |
||
|
|
||
| // --------- | ||
| // URL configuration | ||
|
|
||
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.
curious when do we invalidate ?
I think we should invalidate cache on token change and api endpoint