Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

(#3194) Add command to purge cached queries #3209

Merged
merged 6 commits into from
Jun 20, 2023
Merged

(#3194) Add command to purge cached queries #3209

merged 6 commits into from
Jun 20, 2023

Conversation

AdmiringWorm
Copy link
Member

@AdmiringWorm AdmiringWorm commented Jun 9, 2023 •
edited
Loading

Description Of Changes

The changes in this PR adds a new command to Chocolatey CLI
that allows the user to clear any cached queries that have been saved
on their system.
This will clear both system and user level caches when running as an
administrator, and user level caches when running in a non-elevated
context.

Additionally, the ability to only remove expired caches is added as well
as just listing how many items has been cached.

Motivation and Context

To make it easier for users to clear out any cached items on their system, without having to go to the locations manually.

Testing

What is listed will be highly dependent on your system.

  1. Run choco cache in a non-elevated context.
  2. Ensure the number of cached sources and items are listed for the user level cache only.
  3. Ensure there is a warning about running as an administrator.
  4. Run choco cache in an elevated context.
  5. Ensure the number of cached sources and items are listed for both system and user level caches.
  6. Ensure there is no longer a warning about running as an administrator.
  7. Run choco cache remove in a non-elevated context.
  8. Ensure only the the user level cached items are removed.
  9. Run choco cache remove in a elevated context.
  10. Ensure both system and user level cached items are removed.
  11. Run choco search chocolatey with the chocolatey source enabled in both elevated and non-elevated context.
  12. Run choco cache remove --expired in an elevated context
  13. Ensure no items was removed.
  14. Run choco cache get
  15. Ensure a warning is outputted that get is an unknown command, and list will be used.

Help description check

  1. Run choco -h
  2. Ensure v2.1.0 is listed together with the cache command.
  3. Run choco cache -h
  4. Ensure all values looks appropriate and nothing seems off.

Operating Systems Testing

  • Windows 10

Change Types Made

  • Bug fix (non-breaking change).
  • Feature / Enhancement (non-breaking change).
  • Breaking change (fix or feature that could cause existing functionality to change).
  • Documentation changes.
  • PowerShell code changes.

Change Checklist

  • Requires a change to the documentation.
  • Documentation has been updated.
  • Tests to cover my changes, have been added.
  • All new and existing tests passed?
  • PowerShell code changes: PowerShell v2 compatibility checked?

Related Issue

Fixes #3194

@AdmiringWorm AdmiringWorm self-assigned this Jun 9, 2023
@AdmiringWorm AdmiringWorm requested a review from gep13 June 9, 2023 11:00
@AdmiringWorm AdmiringWorm force-pushed the cache-clearing-command branch 3 times, most recently from f6baa19 to 673aae8 Compare June 15, 2023 16:27
@gep13
Copy link
Member

gep13 commented Jun 16, 2023

I am not sure what this is referring to:

Ensure there is a warning about running as an administrator.

I didn't see any warning. Can you confirm what is expected here?

gep13
gep13 requested changes Jun 16, 2023
View reviewed changes
src/chocolatey/infrastructure.app/commands/ChocolateyCacheCommand.cs Outdated Show resolved Hide resolved
src/chocolatey/infrastructure.app/commands/ChocolateyCacheCommand.cs Outdated Show resolved Hide resolved
src/chocolatey/infrastructure.app/commands/ChocolateyCacheCommand.cs Outdated Show resolved Hide resolved
src/chocolatey/infrastructure.app/commands/ChocolateyCacheCommand.cs Outdated Show resolved Hide resolved
src/chocolatey/infrastructure.app/commands/ChocolateyCacheCommand.cs Outdated Show resolved Hide resolved
src/chocolatey/infrastructure.app/commands/ChocolateyCommandBase.cs Show resolved Hide resolved
src/chocolatey/infrastructure.app/commands/ChocolateyCacheCommand.cs Outdated Show resolved Hide resolved
src/chocolatey/infrastructure.app/commands/ChocolateyCacheCommand.cs Outdated Show resolved Hide resolved
src/chocolatey/infrastructure.app/commands/ChocolateyCacheCommand.cs Outdated Show resolved Hide resolved
src/chocolatey/infrastructure.app/configuration/ChocolateyConfiguration.cs Outdated Show resolved Hide resolved
@gep13 gep13 requested a review from pauby June 16, 2023 07:18
@gep13
Copy link
Member

gep13 commented Jun 16, 2023

@pauby can I get you to put your eyes on this section of the PR: https://github.com/chocolatey/choco/pull/3209/files#diff-798be7ca9a8afe2aa34a870384e9b9bfefa7155703b839c1262e3bc798ca02b2R168-R173 Looking for some feedback on the wording of the command help description. Thanks

@gep13
Copy link
Member

gep13 commented Jun 16, 2023

@AdmiringWorm just thinking about this as well... can we get some tests added to the CacheCommand class, even just verification that the options are available, etc. Similar to what was done with the export command. If there are any easy wins in terms of Pester tests as well, it would be good to get those added in as well.

@AdmiringWorm
(maint) Add structure for outputting help
82049aa 
This changes how the help output is implemented in
Chocolatey CLI.
It adds the handling of outputting the majority of the help
output in the class called ChocolateyCommandBase that can
be used to writing help documentation easier, and instead of
thinking about the structure of the output instead needs to
override specific methods to give the information that is needed.

Some defaults are additionally handled as well to make common
output be easier and more maintainable.
@AdmiringWorm
(maint) Add ability to display version on command
d64de0e 
The changes here allows the displaying of what version
a command was implemented in more easily. THis gives
more information to a user whether a command is available
to them ore not when viewing the documentation on the
website.
@AdmiringWorm AdmiringWorm force-pushed the cache-clearing-command branch from 673aae8 to a4f9f16 Compare June 16, 2023 09:28
@AdmiringWorm AdmiringWorm force-pushed the cache-clearing-command branch from a4f9f16 to 2fd8aa6 Compare June 16, 2023 10:21
@AdmiringWorm AdmiringWorm requested review from gep13 and pauby June 16, 2023 10:21
@AdmiringWorm
Copy link
Member Author

I am not sure what this is referring to:

Ensure there is a warning about running as an administrator.

I didn't see any warning. Can you confirm what is expected here?

When running as in a non-elevated context, a warning will be outputted running as an administrator is required to clean out the system http cache.

@AdmiringWorm AdmiringWorm force-pushed the cache-clearing-command branch 3 times, most recently from 794bc0e to e567d49 Compare June 19, 2023 19:26
@gep13
Copy link
Member

gep13 commented Jun 20, 2023

Running through the tests again, I can't replicate this step:

  1. Ensure there is a warning about running as an administrator.

I see a warning about running as an Administrator when choco cache remove is run, but not when running choco cache list. Can you confirm that this is intended?

@gep13 gep13 force-pushed the cache-clearing-command branch from e567d49 to 24fd61a Compare June 20, 2023 06:59
@gep13 gep13 dismissed pauby’s stale review June 20, 2023 07:00

The requested changes have been made.

gep13
gep13 previously approved these changes Jun 20, 2023
View reviewed changes
Copy link
Member

@gep13 gep13 left a comment

Choose a reason for hiding this comment

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

LGTM!

@gep13
Copy link
Member

gep13 commented Jun 20, 2023

@AdmiringWorm aside from confirmation about Step 3 above, this looks good to me, and I would say is ready to be merged.

@AdmiringWorm
Copy link
Member Author

AdmiringWorm commented Jun 20, 2023 •
edited
Loading

Running through the tests again, I can't replicate this step:

  1. Ensure there is a warning about running as an administrator.

I see a warning about running as an Administrator when choco cache remove is run, but not when running choco cache list. Can you confirm that this is intended?

I don't have any specific reason for not adding it. It just seemed to be useful in a remove scenario. I can also add it for listing the cache stats if it makes sense to have it there.

EDIT: Actually, looking at the steps again. This may have been an oversight on my part. Let me check

@AdmiringWorm
(chocolatey#3194) Add command to purge cached queries
c21f51a 
The changes in this commit adds a new command to Chocolatey CLI
that allows the user to clear any cached queries that have been saved
on their system.
This will clear both system and user level caches when running as an
administrator, and user level caches when running in a non-elevated
context.

Additionally, the ability to only remove expired caches is added as well
as just listing how many items has been cached.
@AdmiringWorm
(maint) Set block body in method as default
2bb90fa 
The change here updates the editorconfig file to default
to using a block body instead of an expression. This helps
when using any of the Visual Studio features when generating
new methods as it will now create a full method without using
expressions for single line methods.
@AdmiringWorm
(maint) Normalize line endings in Command Base class
1d67bcf
@AdmiringWorm
(maint) Include warning messages in docs output
47d9f75
@AdmiringWorm AdmiringWorm force-pushed the cache-clearing-command branch from 24fd61a to 47d9f75 Compare June 20, 2023 12:32
gep13
gep13 approved these changes Jun 20, 2023
View reviewed changes
Copy link
Member

@gep13 gep13 left a comment

Choose a reason for hiding this comment

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

LGTM!

@gep13 gep13 merged commit 45ef807 into chocolatey:develop Jun 20, 2023
@gep13
Copy link
Member

gep13 commented Jun 20, 2023

@AdmiringWorm thank you very much for getting this added!

@AdmiringWorm AdmiringWorm deleted the cache-clearing-command branch September 25, 2023 12:42
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@gep13 gep13 gep13 approved these changes

@pauby pauby pauby left review comments

Assignees

@AdmiringWorm AdmiringWorm

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add command to allow purging of cached HTTP queries
3 participants
@AdmiringWorm @gep13 @pauby