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

(GH-83) Turn off download progress #101

Merged
merged 9 commits into from
Jul 28, 2019

Conversation

jrdnr
Copy link
Contributor

@jrdnr jrdnr commented Dec 3, 2017

Fixes Issue #83 "Turn off download progress"
Choco Versions >10.4 overwhelm log with download Progress data

Added Check for Choco version and add "--no-progress" to the Params list

  • Renamed Params variable for consistency across functions
  • Updated Choco comand execution formatting to be consistent across functions

Added Check for Choco version and add "--no-progress" to the Params list
 - Renamed Params variable for consistency across functions
 - Updated Choco comand execution formatting to be consistent across functions
Adds "[Diagnostics.CodeAnalysis.SuppressMessage('PSAvoidUsingInvokeExpression','')]" to function UninstallPackage, to match other functions that use InvokeExpression to call Chco
Copy link
Member

@ferventcoder ferventcoder left a comment

Choose a reason for hiding this comment

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

Pretty good, but finding the version of Chocolatey is quite a bit simpler, plus there may be times where (depending on how it was installed) there may not be a Chocolatey nupkg in the lib folder.

@jrdnr
Copy link
Contributor Author

jrdnr commented Dec 8, 2017

The reason I used Get-ChocoInstalledPackage instead of choco -v, is because of the work done with caching the package list, running choco -v 15 times takes about 10.3 seconds, where as running Get-ChocoInstalledPackage takes about 1.6. Pushing that up to 45 repetitions and choco -v is around 30 seconds and Get-ChocoInstalledPackage is still only at 1.8 seconds.

I'm sorry I don't know enough about how Choco works to follow what would happen if there were not a "Chocolatey nupkg in the lib folder" would this mean that choco list -lo -r would not return a value for chocolatey?

Powershell needs versions to be specified as strings in order to compair them to version numbers.
Updated to '0.10.4' insure accurate logic.
@ferventcoder
Copy link
Member

I'm sorry I don't know enough about how Choco works to follow what would happen if there were not a "Chocolatey nupkg in the lib folder" would this mean that choco list -lo -r would not return a value for chocolatey?

It depends on how Chocolatey was installed, but folks are free to install it in their own ways and while we wish everything was a good steward about getting the nupkg in the proper place, that isn't always true. Above that, for the longest time even our scripts were not doing it correctly, so some of the configuration managers out there were also implementing it wrong. So we can't assume that the nupkg is there.

The reason I used Get-ChocoInstalledPackage instead of choco -v, is because of the work done with caching the package list, running choco -v 15 times takes about 10.3 seconds, where as running Get-ChocoInstalledPackage takes about 1.6. Pushing that up to 45 repetitions and choco -v is around 30 seconds and Get-ChocoInstalledPackage is still only at 1.8 seconds.

This is a great point. Perhaps cache the version output as well? I think it will be even faster than iterating the cache of installed packages and it will also be more accurate due to the above constraints.

Thoughts?

@jrdnr
Copy link
Contributor Author

jrdnr commented Dec 11, 2017

I would agree, there would be time savings from caching choco -v especially since it wouldn't need to be purged or expired as frequently as Installed Package. The question I wasn't quite sure how to deal with is logic around how to decide if the cached version is still valid, I'm tempted to go with a blunt solution like a 60 second timer, worst case scenario Chocolatey gets updated during a DSC run and any remaining work is done without --no-progress. Is there a reasonable way to know if choco gets upgraded or should I just go with the timer?

Ensured explicitly casting '0.10.4' to Version
@ferventcoder
Copy link
Member

Is there a reasonable way to know if choco gets upgraded or should I just go with the timer?

I think the timer is going to be fine.

Added parsing to split on '-' and only use the first section.
@jrdnr
Copy link
Contributor Author

jrdnr commented Dec 15, 2017

@ferventcoder Let me know if there are any other items that need to be addressed before this can be merged. If this is good to go as implemented I can squash it down and correct the commit messages that are inaccurate.

@pauby
Copy link
Member

pauby commented Oct 2, 2018

@ferventcoder Can you take a look at this and see if there is anything else needed since your review?

@jrdnr
Copy link
Contributor Author

jrdnr commented Oct 9, 2018

Thanks for the bump @pauby would love to get this into into production, would do a lot to clean up logs.
cc. @ferventcoder

@pauby pauby self-requested a review December 10, 2018 21:09
@pauby pauby changed the title Fixes Issue #83 "Turn off download progress" (GH-83) Turn off download progress Dec 12, 2018
Copy link
Member

@pauby pauby left a comment

Choose a reason for hiding this comment

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

@jrdnr I've gone through and resolved the conversations by @ferventcoder - just one small point from me, where we can use Advanced Functions. After that I'm happy to give this a spin and merge.

@pauby pauby requested a review from ferventcoder April 1, 2019 10:30
@pauby pauby merged commit aed5976 into chocolatey:development Jul 28, 2019
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