Skip to content

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

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

Merged
merged 5 commits into from
Apr 1, 2025

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

@zeusongit
Copy link
Contributor

@reddyashish Can you take a look at the failing smoke test.

@reddyashish reddyashish removed the WIP label Apr 1, 2025
@reddyashish
Copy link
Contributor Author

This is ready to review. Fixed all the edge cases and now it dynamically computes the python package dependency based on the python node engine.

@zeusongit
Copy link
Contributor

Who is @DreadPirateRoberts90 ?

@reddyashish
Copy link
Contributor Author

Haha, that was my different account which i did not notice on my windows instance.

@zeusongit zeusongit merged commit 2f57b87 into DynamoDS:master Apr 1, 2025
21 checks passed
github-actions bot pushed a commit that referenced this pull request Apr 1, 2025
Copy link

github-actions bot commented Apr 1, 2025

Successfully created backport PR for RC3.5.0_master:

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.

3 participants