[DYN-2609] Fix crash when installing empty package #10561
Conversation
| !are_contents_empty && x.contents.Contains(PackageManagerClient.PackageContainsBinariesConstant); | ||
| var contains_python = | ||
| !are_contents_empty && x.contents.Contains(PackageManagerClient.PackageContainsPythonScriptsConstant); | ||
| return contains_binaries || contains_python; |
There was a problem hiding this comment.
PackageManagerClient.PackageContainsPythonScriptsConstant and PackageManagerClient.PackageContainsBinariesConstant are deprecated so I'm not sure if we should use these or something else.
There was a problem hiding this comment.
I'm not sure either, but the package header only contains one boolean field -contains binaries which is true if the package contains binaries or python...
There was a problem hiding this comment.
There are 41 versions across 22 packages that are still using this "obsolete" method of marking packages. We could theoretically update those version records in the database to have the contains_binaries field set to true and then we could remove these constants.
There was a problem hiding this comment.
Is there a contains_python field as well? If so, then we need to add it to the check here.
There was a problem hiding this comment.
No, there is only a single field for both.
|
@alfredo-pozo the Any clue? |
|
The last execution is completed, the failures were because of Docker issues @aparajit-pratap |
|
LGTM |
Purpose
Fix for regression caused by #10544 - crash when installing an empty package like LunchBox. Changes to new package version API's on the PM (I think) return the
PackageVersionwhose contents can benullfor an empty package. Prior to the API change, the contents would be an empty string for an empty package. Thus it was necessary to add a null check.The UI automation test DynamoTests.dll.DynamoTests.Tests.SmokeTestPackage was failing, which signaled this regression.
Declarations
Check these if you believe they are true
*.resxfilesReviewers
@mmisol
@mjkkirschner
FYIs
@alfredo-pozo