-
Notifications
You must be signed in to change notification settings - Fork 4
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
feat: Improve Git handler and introduce caching #1689
Conversation
d3fd5fe
to
0a61036
Compare
0a61036
to
bc03a70
Compare
The generated OpenAPI client is not up to date with the latest changes in the OpenAPI specification. |
bc03a70
to
595f7ba
Compare
The generated OpenAPI client is not up to date with the latest changes in the OpenAPI specification. |
595f7ba
to
0a6a9fb
Compare
A Storybook preview is available for commit 90c9982. |
The generated OpenAPI client is not up to date with the latest changes in the OpenAPI specification. |
2fb7b36
to
d6fcb0f
Compare
ed83167
to
4532c46
Compare
The generated OpenAPI client is not up to date with the latest changes in the OpenAPI specification. |
4532c46
to
db15ea2
Compare
The generated OpenAPI client is not up to date with the latest changes in the OpenAPI specification. |
db15ea2
to
47e88ed
Compare
The generated OpenAPI client is not up to date with the latest changes in the OpenAPI specification. |
3f67356
to
8caa705
Compare
- key: redis-config | ||
path: redis.conf | ||
containers: | ||
- name: {{ .Release.Name }}-backend-redis |
Check warning
Code scanning / SonarCloud
Storage limits should be enforced
b32b916
to
bbc3f52
Compare
This commit requests changes on the PR #1689. In particular, the following has changed: - Replace cache key from path and revision with git_model_id (database id). The unique database ID of the Git model in the Collaboration Manager has to be part of the cache key. For safety reasons, I'd not share a cache between projects even though they link to the same repository. This doesn't really matter currently since we make new requests against the Git API every time, but we can't assure that for the future. - An attack could look like: - Project 1 links Git repository "https://example.com" to a model. - Project 2 links the same Git repository to a model with a different token. - The token of project 1 expires. Project 2 keeps updating the cache with a valid token. - Users of project 1 can still access the cache for project 2 without a valid token. - Since a git_model only has one revision, it's safe to drop the revision for artifacts. The revision is only relevant for files from the repository. In those cases, the value was passed as parameter every time. Therefore, I've removed it from the Cache object. - For license reasons, I switched from Redis to Valkey. - Use a Kubernetes secret for the valkey password. - For REST compatibility, I've changed the resource name of "empty_cache" to "cache". - I've reenabled the model badge error handling. - Added a new function loadDiagramCacheMetadata in model-diagram-dialog.component.ts to reduce duplicated code. - Added the job id to the diagram cache code snippet. - Changed the styling of the "Clear cache" button.
614dffb
to
85003a2
Compare
The generated OpenAPI client is not up to date with the latest changes in the OpenAPI specification. |
This commit requests changes on the PR #1689. In particular, the following has changed: - Replace cache key from path and revision with git_model_id (database id). The unique database ID of the Git model in the Collaboration Manager has to be part of the cache key. For safety reasons, I'd not share a cache between projects even though they link to the same repository. This doesn't really matter currently since we make new requests against the Git API every time, but we can't assure that for the future. - An attack could look like: - Project 1 links Git repository "https://example.com" to a model. - Project 2 links the same Git repository to a model with a different token. - The token of project 1 expires. Project 2 keeps updating the cache with a valid token. - Users of project 1 can still access the cache for project 2 without a valid token. - Since a git_model only has one revision, it's safe to drop the revision for artifacts. The revision is only relevant for files from the repository. In those cases, the value was passed as parameter every time. Therefore, I've removed it from the Cache object. - For license reasons, I switched from Redis to Valkey. - Use a Kubernetes secret for the valkey password. - For REST compatibility, I've changed the resource name of "empty_cache" to "cache". - I've reenabled the model badge error handling. - Added a new function loadDiagramCacheMetadata in model-diagram-dialog.component.ts to reduce duplicated code. - Added the job id to the diagram cache code snippet. - Changed the styling of the "Clear cache" button.
8b89310
to
0e57c16
Compare
@MoritzWeber0 I went over your comments and implemented them in my last commit. I mentioned open issues in my replies to your comments, but the main open issue is how to handle cache clearing (see my comment above), and what resource we should allocate to the valkey pod. |
This commit requests changes on the PR #1689. In particular, the following has changed: - Replace cache key from path and revision with git_model_id (database id). The unique database ID of the Git model in the Collaboration Manager has to be part of the cache key. For safety reasons, I'd not share a cache between projects even though they link to the same repository. This doesn't really matter currently since we make new requests against the Git API every time, but we can't assure that for the future. - An attack could look like: - Project 1 links Git repository "https://example.com" to a model. - Project 2 links the same Git repository to a model with a different token. - The token of project 1 expires. Project 2 keeps updating the cache with a valid token. - Users of project 1 can still access the cache for project 2 without a valid token. - Since a git_model only has one revision, it's safe to drop the revision for artifacts. The revision is only relevant for files from the repository. In those cases, the value was passed as parameter every time. Therefore, I've removed it from the Cache object. - For license reasons, I switched from Redis to Valkey. - Use a Kubernetes secret for the valkey password. - For REST compatibility, I've changed the resource name of "empty_cache" to "cache". - I've reenabled the model badge error handling. - Added a new function loadDiagramCacheMetadata in model-diagram-dialog.component.ts to reduce duplicated code. - Added the job id to the diagram cache code snippet. - Changed the styling of the "Clear cache" button.
0e57c16
to
ab5061c
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
d15f1e3
to
e5fa387
Compare
This comment has been minimized.
This comment has been minimized.
e5fa387
to
3d67ea4
Compare
This comment has been minimized.
This comment has been minimized.
- Add vertical autoscaler to track resource usage of valkey - Increase TTL for valkey data - Clear cache when git model is updated - Mount data and config properly to valkey - Fallback to traditional loading if saving or loading in valkey fails
3d67ea4
to
9bc25bd
Compare
Quality Gate passedIssues Measures |
This report was generated by comparing 90c9982 with 7e18c2d. ArtifactName:
|
item | count |
---|---|
pass | 242 |
change | 16 |
new | 4 |
delete | 0 |
📝 Report
Differences
Model Components_Model Diagram Cache_Combined_desktop.png
actual | |
---|---|
expected | |
difference |
Model Components_Model Diagram Cache_Combined_mobile.png
actual | |
---|---|
expected | |
difference |
Model Components_Model Diagram Cache_Diagram Error_desktop.png
actual | |
---|---|
expected | |
difference |
Model Components_Model Diagram Cache_Diagram Error_mobile.png
actual | |
---|---|
expected | |
difference |
Model Components_Model Diagram Cache_Diagram Loaded_desktop.png
actual | |
---|---|
expected | |
difference |
Model Components_Model Diagram Cache_Diagram Loaded_mobile.png
actual | |
---|---|
expected | |
difference |
Model Components_Model Diagram Cache_Diagram Not Loaded_desktop.png
actual | |
---|---|
expected | |
difference |
Model Components_Model Diagram Cache_Diagram Not Loaded_mobile.png
actual | |
---|---|
expected | |
difference |
Model Components_Model Diagram Cache_Loading Without Scroll_desktop.png
actual | |
---|---|
expected | |
difference |
Model Components_Model Diagram Cache_Loading Without Scroll_mobile.png
actual | |
---|---|
expected | |
difference |
Model Components_Model Diagram Cache_Loading_desktop.png
actual | |
---|---|
expected | |
difference |
Model Components_Model Diagram Cache_Loading_mobile.png
actual | |
---|---|
expected | |
difference |
Model Components_Model Diagram Cache_No Diagram For Filter_desktop.png
actual | |
---|---|
expected | |
difference |
Model Components_Model Diagram Cache_No Diagram For Filter_mobile.png
actual | |
---|---|
expected | |
difference |
Model Components_Model Diagram Cache_No Diagram_desktop.png
actual | |
---|---|
expected | |
difference |
Model Components_Model Diagram Cache_No Diagram_mobile.png
actual | |
---|---|
expected | |
difference |
New Items
Model Components_Model Diagram Code Block_Code Block With Job ID_desktop.png
Model Components_Model Diagram Code Block_Code Block With Job ID_mobile.png
Model Components_Model Diagram Code Block_Code Block With Token_desktop.png
Model Components_Model Diagram Code Block_Code Block With Token_mobile.png
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 appreciate all the work that went into this PR. It not only makes loading the diagram cache and model badges faster, but also lays the foundation for future caching in the application. Thanks for that! 🥳
Breaking Changes
A new key
.Values.valkey.password
is required. Fill it with a randomly generated password.Changes
{git path}:{git revision}:f:{file path}
for files and{git path}:{git revision}:a:{job id}:{file_path}
for artifacts. These keys should uniquely identify a file or artifact, and the hierarchical structure allows us to easily clear the cache for certain git models (i.e., we can remove all keys under{git path}:{git revision}:*
).Resolves #893
TODO: