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

imagepruning: temporarily disable event handling #87

Closed

Conversation

miminar
Copy link

@miminar miminar commented Jun 28, 2018

Underlying gonum/graph assigns conflicting IDs to new nodes after node
deletion. Thus two different nodes amy be assigned the same ID. This
leads to panic when an edge is set between them.

Temporarily disabling event handling until the package is fixed. This
ensures that no new nodes will be added to the graph after the first
node gets removed.

deads2k and others added 30 commits June 27, 2018 13:42
@sjenning you probably want to revert this and find the real wiring problem
deads2k and others added 22 commits June 27, 2018 16:20
Underlying gonum/graph assigns conflicting IDs to new nodes after node
deletion. Thus two different nodes amy be assigned the same ID. This
leads to panic when an edge is set between them.

Temporarily disabling event handling until the package is fixed. This
ensures that no new nodes will be added to the graph after the first
node gets removed.

Signed-off-by: Michal Minář <miminar@redhat.com>
@liggitt
Copy link

liggitt commented Jun 28, 2018

Underlying gonum/graph assigns conflicting IDs to new nodes after node
deletion.

That doesn't seem unreasonable. Are we retaining handles to node ids somewhere other than edges maintained by the graph?

@deads2k
Copy link
Owner

deads2k commented Jun 28, 2018

I picked this. @bparees you and @miminar may need to access impact to 3.10

@deads2k deads2k closed this Jun 28, 2018
@miminar
Copy link
Author

miminar commented Jun 28, 2018

That doesn't seem unreasonable. Are we retaining handles to node ids somewhere other than edges maintained by the graph?

We take references to whole nodes and keep them until the end. I don't see how that should prevent us from connecting newly added nodes once a single node is deleted from the graph (which is modifiable and allows for node removals).

@deads2k
Copy link
Owner

deads2k commented Jun 28, 2018

There's a unit test you'll need to remove too.

@miminar
Copy link
Author

miminar commented Jun 28, 2018

There's a unit test you'll need to remove too.

Yes, sorry about that. Here's updated commit: miminar@706c1b3

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