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

Reconcile getModules With Library Preloading #1863

Closed
3 tasks
radeusgd opened this issue Jul 14, 2021 · 0 comments
Closed
3 tasks

Reconcile getModules With Library Preloading #1863

radeusgd opened this issue Jul 14, 2021 · 0 comments
Labels
p-high Should be completed in the next sprint

Comments

@radeusgd
Copy link
Member

Summary

Once the imports started to be done lazily, the getModules method may return None for modules that do exist but are simply not loaded.

One example of this causing issues was visualizations not loading their context modules, which was quick-fixed by adding the explicit ensurePackageIsLoaded before executing the visualization (#1857).

Another potential point for problems is the openFile endpoint (see OpenFileCmd) - if the module the file is bound to is not found, it will most likely fail to open that file properly (or worse, although now I think this should not happen, associate it with an empty module and lead to clashes when that module is actually loaded).

We need to go through all usages of getModule and consult each of them with a team member responsible for a given component to analyse if it should try to preload the module first or not.

Possibly, we may just implement a 'catchall' solution that just always ensures that the module is preloaded on each getModule call, but we need to analyse if it is safe to do so.

Value

  • We avoid potential bugs introduced by making the imports lazily.

Specification

  • Go through each usage of getModule, most likely on a call with @iamrecursion, @4e6 and @kustosz and analyse what is the right decision for each of them (if they should try to preload libraries if it was not yet loaded).
  • Depending on decisions made, modify the implementation accordingly.
  • If reasonably possible, at tests for the affected scenarios.

Acceptance Criteria & Test Cases

  • The above specification has been completed.
@radeusgd radeusgd added Category: Backend p-high Should be completed in the next sprint labels Jul 14, 2021
@wdanilo wdanilo closed this as completed Apr 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p-high Should be completed in the next sprint
Projects
None yet
Development

No branches or pull requests

2 participants