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

Potential non imported node delete solve #3

Closed
wants to merge 1 commit into from

Conversation

samuelbray32
Copy link

Description

Within bridge restrict, was hitting error if table 2 not defined in the namespace. This change loads the free table from the connection and assigns it to the node, allowing for the restriction to be passed.

In testing this allowed me to remove the non part tables from the imports in SpyglassMixin._import_part_masters() and not hit errors in delete

I might be missing an issue this would cause in further restriction propagation or other use case, so feel free to reject if so

@CBroz1
Copy link
Owner

CBroz1 commented Jun 26, 2024

Thanks for your work @samuelbray32!

This PR could potentially contribute to spy #1002 by skipping nodes that we would otherwise need to add to _import_part_masters in the future. I'd like to roll it out as-is and then check in with user intuition about how unimported nodes should be treated

  1. Ignored, as done here - potentially hitting the DJ error 1002 was designed to avoid
  2. Raised as error, to explicitly cascade through all of spyglass

If the former, we can revisit this approach

@CBroz1 CBroz1 closed this Jun 26, 2024
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.

2 participants