Skip to content
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

Option to disable fallback renderer if a diagram cache is in use #287

Closed
Wuestengecko opened this issue May 9, 2023 · 2 comments · Fixed by #291
Closed

Option to disable fallback renderer if a diagram cache is in use #287

Wuestengecko opened this issue May 9, 2023 · 2 comments · Fixed by #291

Comments

@Wuestengecko
Copy link
Member

We occasionally see issues where diagrams aren't properly put into or pulled from the pre-rendered diagram image cache. In some instances, this causes the (internally rendered) diagrams to look just off enough to cause hard-to-spot issues down the line, especially if people aren't paying close attention because they think everything went fine.

We should add more tools to better deal with this type of situation. My first proposal would be to add an option that entirely disables the fallback renderer if a diagram cache was configured for a model. This could (should?) be made the default, so that people who want the fallback have to explicitly opt into it.

My proposal would be to add an environment variable like CAPELLAMBSE_FALLBACK_RENDER_AIRD, which, if absent (the default), will disable the fallback renderer. Alternatively, we could also add a model argument, which would make it configurable on a per-model basis.

The variable should not have any effect if no diagram_cache argument was given, i.e. if there is no cache to pull images from, we will always render them internally.

Care needs to be taken so that this change does not negatively impact diagrams that are always internally rendered, such as ones made with capellambse-context-diagrams.

@vik378
Copy link
Member

vik378 commented May 9, 2023

the fallback renderer is still a thing in a few usecases, I think we shouldn't make it difficult to use. I'd rather see this as a per-model option and also only disable it given the cache (enabled by default) so that we dont end up with no means to render

@Wuestengecko
Copy link
Member Author

Wuestengecko commented May 17, 2023

I don't want to make it difficult to use either, I just want it to be disabled by default for AIRD diagrams :)

And yes, if there's no cache specified at all then we will always need to use the internal engine, otherwise we wouldn't be able to display anything.

So my understanding of your point is:

  • If there's a cache, disable the AIRD renderer unless an additional argument was passed during model loading
  • If there's no cache, keep the renderer enabled
  • In any case, always enable the renderer for diagrams that did not come from an AIRD file

That sounds good to me.

Wuestengecko added a commit that referenced this issue May 23, 2023
Experience has shown that it's more often than not undesirable to have
the internal rendering engine jump in for diagrams that could not be
found in the cache. This commit arranges for the internal engine to be
disabled for AIRD diagrams by default if a cache was specified. This
behavior can be overridden by setting the new ``fallback_render_aird``
model load parameter to ``True``, which re-enables the rendering engine
for all diagram types.

Diagrams that do not come from one of the model's AIRD files are not
affected, and will always fall back to the internal engine. This means
that e.g. context-diagrams, which are generated on-the-fly, will
continue to work as expected, without the need to pre-generate them.

Resolves #287.
Wuestengecko added a commit that referenced this issue May 23, 2023
Experience has shown that it's more often than not undesirable to have
the internal rendering engine jump in for diagrams that could not be
found in the cache. This commit arranges for the internal engine to be
disabled for AIRD diagrams by default if a cache was specified. This
behavior can be overridden by setting the new ``fallback_render_aird``
model load parameter to ``True``, which re-enables the rendering engine
for all diagram types.

Diagrams that do not come from one of the model's AIRD files are not
affected, and will always fall back to the internal engine. This means
that e.g. context-diagrams, which are generated on-the-fly, will
continue to work as expected, without the need to pre-generate them.

Resolves #287.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants