fix: removing duplicated nuget packages folder #746
Conversation
pranav-firake
changed the title
|
Please add how this was tested. |
| packagesFolderName | ||
| ) | ||
| if (!fs.existsSync(packagesFolder)) { | ||
| this.logging.log('Cannot find nuget packages folder in source code') |
There was a problem hiding this comment.
+1. If we invert the if logic, we can simplify this method. Then reducing the logging becomes natural.
There was a problem hiding this comment.
agreed, it is causing unnecessary logs, will remove this.
will add just return here instead of logging
| ) | ||
| if (fs.existsSync(packagesFolder)) { | ||
| fs.rmSync(packagesFolder, { recursive: true, force: true }) | ||
| this.logging.log(`Removed nuget packages folder: ${packagesFolder}`) |
There was a problem hiding this comment.
One more thing: In the log message, we should say why it is removed (e.g. "Removed packages folder {packagesFolder} from directory containing artifacts to be uploaded because it is a duplicate of {nugetFolder}").
Going forward, we should be mindful of the messages we log. They should provide enough context so the reader understands the intended behavior without having any knowledge of the code itself.
There was a problem hiding this comment.
thanks for suggestion, updated logging message accordingly
… to be copied before zipping
| @@ -41,6 +43,21 @@ export class ArtifactManager { | |||
| } | |||
| } | |||
|
|
|||
| async removeDuplicateNugetPackagesFolder(request: StartTransformRequest) { | |||
There was a problem hiding this comment.
as a potential optimization, is it possible to skip adding these files to the zip in the first place. If I understand correctly right now we spend extra time copying these files in the step before only to end up going through the folder again and deleting them. If we could create the artifacts in a single go that would also make the process more efficient. Is this possible?
There was a problem hiding this comment.
yes Ege, thanks for suggestion.
After this hotfix, we will take a stab in detail on optimizing entire zipping with ticket P194692593.
|
Small reminder to remove the |
pranav-firake
changed the title
pranav-firake
deleted the
pranavfi-fix-for-deduping-nuget-packages-folder
branch
Problem
V1642391964
Currently when we are packaging artifacts.zip on the IDE, the nuget packages are part of
This is causing increased artifacts size.
Solution
we are not using packages folder from sourceCode for transformations in Qnet CLI, we are only using nuget packages from references folder.
So we can safely delete packages folder before zipping to save on space.
Simple example on saving space is
Sample applications - Bobs Bookstore
Size of packages folder - 241MB
size of Artifacts zip BEFORE below changes of deduping - 179 MB
size of Artifacts zip AFTER below changes of deduping - 89 MB
we are saving almost 45% space in artifacts zipping and thereby for uploads. This will also benefit for customer issues with large codebase
UPDATE :
Also found that copyFile is async and we are zipping / deleting package folder even before all copying is complete. so changed copyFile to Sync operation
Testing
Case : Updated a BobsBookstore application to bloat to 2.3 GB of artifacts.zip.
with deduping, for same project artifacts size is - 1.2 GB
License
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.