Dynamo
Dynamo copied to clipboard
DynamoDS
DYN-8274 PythonScript With Dispose when Exception
Purpose
This test is validating that if there is an exception inside the with statement the resources are disposed correctly. The Python.Runtime.dll is needed due that we need to changes done in pythonnet project otherwise the test will be failing
Declarations
Check these if you believe they are true
- [X] Is documented according to the standards
- [X] The level of testing this PR includes is appropriate
- [X] User facing strings, if any, are extracted into
*.resxfiles - [ ] 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
- [ ] This PR introduces new feature code involve network connecting and is tested with no-network mode.
Release Notes
This test is validating that if there is an exception inside the with statement the resources are disposed correctly
Reviewers
@aparajit-pratap @zeusongit
FYIs
Why is Python.Runtime.dll modified, was there also a change made to CPython3? I thought the change was only for PythonNet3 engine?
Why is Python.Runtime.dll modified, was there also a change made to CPython3? I thought the change was only for PythonNet3 engine?
The code fix was done in the pythonnet solution, specifically in the Python.Runtime project, then the only way which I found to make the test to pass is by updating the Python.Runtime.dll in ...Dynamo\extern\Python (otherwise my code is not present and getting an error).
Not sure if the dll should be included or not in this PR or if my PR for pythonnet should be merged first and then create this PR (in some way the Python.Runtime.dll should be updated with my changes).
Hey @RobertGlobant20 I did look into this, and I think the fix should end up in PythonNet3Engine repo, the package that adds the PythonNet3 engine also has a Python.Runtime.dll. And from the issue, it seems the bug was not reproduced with CPython3.
Hey @RobertGlobant20 I did look into this, and I think the fix should end up in PythonNet3Engine repo, the package that adds the PythonNet3 engine also has a
Python.Runtime.dll. And from the issue, it seems the bug was not reproduced with CPython3.
The fix was made in pythonnet repo. I think PythonNet3Engine repo is the Dynamo package, not the Python net implementation.
Hey @RobertGlobant20 I did look into this, and I think the fix should end up in PythonNet3Engine repo, the package that adds the PythonNet3 engine also has a
Python.Runtime.dll. And from the issue, it seems the bug was not reproduced with CPython3.The fix was made in pythonnet repo. I think PythonNet3Engine repo is the Dynamo package, not the Python net implementation.
Both packages consumes the pythonnet repo, so I guess the question is which package/engine needs the fix?
Hey @RobertGlobant20 I did look into this, and I think the fix should end up in PythonNet3Engine repo, the package that adds the PythonNet3 engine also has a
Python.Runtime.dll. And from the issue, it seems the bug was not reproduced with CPython3.The fix was made in pythonnet repo. I think PythonNet3Engine repo is the Dynamo package, not the Python net implementation.
Both packages consumes the pythonnet repo, so I guess the question is which package/engine needs the fix?
I suppose we need to check if this PR needs to be made to both dynamo_master and dynamo_py3 branches?
seems that the job passed successfully here: https://master-5.jenkins.autodesk.com/job/Dynamo/job/DynamoSelfServe/job/pullRequestValidation/18088/
@aparajit-pratap based in our slack conversation about the different warning when selecting the CPython3 engine vs the PythonNet3 engine when there is an exception inside the with statement, it's ok to merge this PR to master?
Also is important to mention that the Python.Runtime.dll was generated using the pythonnet.15 solution and using the dynamo_master branch selecting the configuration ReleaseWinPy3