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

Extension point resolution too eager in Envisage 6.0.0 #417

Closed
mdickinson opened this issue Jun 8, 2021 · 7 comments
Closed

Extension point resolution too eager in Envisage 6.0.0 #417

mdickinson opened this issue Jun 8, 2021 · 7 comments

Comments

@mdickinson
Copy link
Member

mdickinson commented Jun 8, 2021

A project using Envisage 6.0.0 has encountered a serious regression: with Envisage 5.0.0 and earlier versions, when starting an Envisage-based application, all plugins were started before extension point contributions between plugins were resolved. With Envisage 6.0.0, some extension point contributions are resolved while plugins are starting. This means that a plugin that contributes to another extension point can no longer assume that it has already been started, and can no longer depend on traits that happen to be initialised by the start method.

(As a side note, initialising traits in the start method is something of an antipattern; that doesn't make this any less of a regression.)

The bug appears to have been introduced as a result of #354.

I don't yet have a shareable reproducing example, but I'm working on it.

@rahulporuri
Copy link
Contributor

In the short term (this week), we'll revert #354 and push out a bugfix release.

@mdickinson
Copy link
Member Author

Thanks, @rahulporuri. That should give us some breathing space to add new tests and figure out what the right approach is.

@rahulporuri
Copy link
Contributor

@mdickinson in the meanwhile, it would be good to have a reproducible example, if you already have one.

@mdickinson
Copy link
Member Author

Yep, I'll see if I can come up with a test. It's probably easier to create an example from scratch than to cut down the project-based example that I have.

rahulporuri pushed a commit that referenced this issue Jun 14, 2021
* Add failing test

* STY: Fix line spacing issues

Co-authored-by: Poruri Sai Rahul <[email protected]>
rahulporuri pushed a commit that referenced this issue Jun 17, 2021
* Add failing test

* STY: Fix line spacing issues

Co-authored-by: Poruri Sai Rahul <[email protected]>
rahulporuri pushed a commit that referenced this issue Jun 17, 2021
* Revert PR #354 (#422)

* Revert "Support observe("name:items") for ExtensionPoint [Requires Traits 6.1] (#354)"

This reverts commit d62af08.

* TST/FIX : Revert test that used observe to listen to changes to

extension point items. The test now uses a static trait change handler
like before

	modified:   envisage/tests/test_plugin.py

* Regression test for #417 (#421)

* Add failing test

* STY: Fix line spacing issues

Co-authored-by: Poruri Sai Rahul <[email protected]>

* Fix test suite dependence on ipykernel (#423)

* Fix hard test suite dependence on ipykernel

* Update envisage/tests/test_ids.py

Co-authored-by: Poruri Sai Rahul <[email protected]>

Co-authored-by: Poruri Sai Rahul <[email protected]>

* DOC : Update changelog

	modified:   CHANGES.rst

Co-authored-by: Mark Dickinson <[email protected]>
@mchilvers
Copy link
Contributor

mchilvers commented Nov 20, 2022

I know this is a bit LTTP but lazy loading in envisage is a fundamental design principle so this one seems a bit scary.

@mdickinson
Copy link
Member Author

@mchilvers Indeed. The offending PR was reverted in short order and a bugfix release made, so lazy loading remains. What's left is to figure out how to redo the changes in #354 in a sensible way.

@mdickinson
Copy link
Member Author

Since the issue described here no longer exists, I'll close here. I've re-opened the original issue #291 that #354 attempted to solve.

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

Successfully merging a pull request may close this issue.

3 participants