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

[DYN-8392] Python dependencies in workspace references. #15948

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

reddyashish
Copy link
Contributor

Purpose

https://jira.autodesk.com/browse/DYN-8392

Python engine dependency in workspace references should update when the user changes the python engine and saves it.

Declarations

Check these if you believe they are true

  • The codebase is in a better state after this PR
  • Is documented according to the standards
  • The level of testing this PR includes is appropriate
  • User facing strings, if any, are extracted into *.resx files
  • All tests pass using the self-service CI.
  • Snapshot of UI changes, if any.
  • Changes to the API follow Semantic Versioning and are documented in the API Changes document.
  • This PR modifies some build requirements and the readme is updated
  • This PR contains no files larger than 50 MB

Release Notes

(FILL ME IN) Brief description of the fix / enhancement. Use N/A to indicate that the changes in this pull request do not apply to Release Notes. Mandatory section

Reviewers

@zeusongit @aparajit-pratap

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the ticket for this pull request: https://jira.autodesk.com/browse/DYN-8392

@@ -804,6 +805,18 @@ private List<INodeLibraryDependencyInfo> ComputeNodeLibraryDependencies()
{
var collected = GetNodePackage(node);

// Handle python nodes explicitly and use the collected node package for those node type.
if (node.ToString().Equals(pythonNodeNamespace) && collected != null)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checking this instead of the node type because the core cannot have dependency reference with PythonNodeModels project.

@@ -187,10 +187,6 @@ internal void DependencyRegen(WorkspaceModel ws, bool forceCompute = false)
HasDependencyIssue = string.IsNullOrEmpty(info.Path);
}

var pythonPackageDependencies = ws.OnRequestPackageDependencies();
Copy link
Contributor Author

@reddyashish reddyashish Mar 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed this as the OnRequestPackageDependencies was just setting the ironpython package details with a fixed version if the workspace has a ironpython2 node. Now, we will compute the dependency package directly when the engine is changed and the script is saved. Should handle Cpython and pythonnet3 cases as well.

@zeusongit
Copy link
Contributor

Worth adding a test?

@zeusongit zeusongit added this to the 3.5 milestone Mar 19, 2025
@reddyashish
Copy link
Contributor Author

@zeusongit There is a already a similar test and I modified that to verify the dependency change when the python engine is modified.

@zeusongit
Copy link
Contributor

There are some test failures, let's retry merging master into your branch

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