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

install internal dependencies before calling post import blocks #352

Conversation

g-arjones
Copy link
Contributor

@doudou I wanted your thoughts before fixing the tests...

Post import blocks may have internal dependencies (i.e python activation code) so these must be installed before calling the blocks. I have delayed the processing of the blocks until all the packages have been processed so we can install the internal dependencies and call all the blocks at once afterward. I don't see issues.

The other possibility would be to leave the calling of post-import blocks alone and move the installation of internal dependencies to Ops::Import#process_post_import_blocks (would have to keep a cache to avoid calling Manager#install on the same packages more than once). That would mean potentially installing osdeps several times rather than installing all internal dependencies at once.

What do you think?

@g-arjones
Copy link
Contributor Author

So, I thought about this some more and neither will work. We really need to call the post-import blocks while still importing packages in order to be able to add more dependencies (by i.e enabling utilities) and installing osdeps while importing really doesn't work well because:

  1. It may happen in multiple threads
  2. Sudo password prompts will probably get lost in the middle of the progress messages

Since we also can't know what packages will be added to the workspace until the whole import operation is done because more packages may be brought in as dependencies (from i.e manifest files), there's no practical solution that I can't think of.

The best thing we can do to work around this at least for the current use case (ensure that python is activated before Autobuild::Python#python_path is called) is to activate python in Autobuild::Python#update_environment directly.

@g-arjones g-arjones closed this Nov 23, 2021
@g-arjones g-arjones deleted the install_internal_dependencies_before_post_import_blocks branch November 23, 2021 17:28
@doudou
Copy link
Member

doudou commented Nov 23, 2021

Not sure if it helps, but there is also a post-update hook in CLI::Main (https://github.com/rock-core/rock.jupyter-package_set/blob/master/init.rb)

@g-arjones
Copy link
Contributor Author

It doesn't unfortunately because those hooks are only called once everything was already loaded (including the environment that requires python to be installed).

See #353

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 this pull request may close these issues.

None yet

2 participants