-
Notifications
You must be signed in to change notification settings - Fork 5
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: Disable the auto-refresh of the semantic browser on Capella session startup #321
feat: Disable the auto-refresh of the semantic browser on Capella session startup #321
Conversation
5021640
to
6eacceb
Compare
The pipeline fails because of a new incompatibility (with pre-commit 4.0.0) of the pre-commit hook docformatter. The issue is from yesterday and open: PyCQA/docformatter#289 |
f5bdbc5
to
2acffc4
Compare
Can you add a small section in https://dsd-dbs.github.io/capella-dockerimages/capella/base/ where you explain that the Semantic Browser is disabled by default, why it is the case and how to enable it again? |
f607246
to
e84597f
Compare
d53e776
to
c4c9b69
Compare
c4c9b69
to
29378d5
Compare
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 refactored some parts of the code in a separate commit and added documentation. Please have another look at my changes, but otherwise: LGTM, thanks for the implementation!
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.
Some findings that let me request changes.
29378d5
to
0fcfa38
Compare
# SPDX-FileCopyrightText: Copyright DB InfraGO AG and contributors | ||
# SPDX-License-Identifier: Apache-2.0 | ||
import fileinput |
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.
We may want to add a module docstring that explains, why we do the string replacement and do not modify using lxml
.
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.
Do you want to write it? You have looked into lxml more deeply and could explain better why it's not a good option for us.
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.
Updated some conversations around the .py file
0fcfa38
to
62e7de6
Compare
Make it possible to disable the auto-refresh of the semantic browser by setting the environment variable `CAPELLA_DISABLE_SEMANTIC_BROWSER_AUTO_REFRESH` to `1`.
- Replace hardcoded workspace directory with env variable - Use functions for better readibility - Use context manager and `enumerate` for file iteration - Use `replace` statement to replace old with new
Describe why we cannot use `lxml` to edit the file in-place and why we use `fileinput` instead.
b0b143a
to
3dd0d49
Compare
When the semantic browser is in the foreground and the displayed setting is active in its toolbar, the UI can freeze for bigger models and every change of model element selection.
It takes its time to load the context of related elements and this bootle neck makes bigger models nearly unusable espoecially for models on a remote Team4Capella (T4C) server.
This is why it has been decided to introduce an according configuration option that disables this toolbar option for every new Capella session.
The default of Capella for a freshly created workspace makes this auto-refresh active as illustrated.
This PR adds a new environment variable which allows to deviate from the default and disables the auto-refresh of the semantic browser, when the environment variables is set to
CAPELLA_DISABLE_SEMANTIC_BROWSER_AUTO_REFRESH=1
.Note, that the toolbar of the semantic browser comes with another button related to the refresh of the semantic browser's view.
The button with curved and yellow arrows allows to trigger a manual refresh of the semantic browser.
Resolves #316