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

feat: add "modinfo_filtered_modules" to collect the filtered modules information #3447

Merged
merged 3 commits into from
Jun 30, 2022
Merged

Conversation

huali027
Copy link
Contributor

@huali027 huali027 commented Jun 21, 2022

  • Add new spec "modinfo_modules" to collect the filtered modules
  • Add another new spec "modinfo_filtered_modules" to run "modinfo filtered_modules" since the result will be filtered if the spec is filterable, and then other info without the filter words will be ignored, which is not expected.

Signed-off-by: Huanhuan Li [email protected]

All Pull Requests:

Check all that apply:

  • Have you followed the guidelines in our Contributing document, including the instructions about commit messages?
  • Is this PR to correct an issue?
  • Is this PR an enhancement?

@huali027
Copy link
Contributor Author

@xiangce Please help to review, thanks.

@huali027 huali027 changed the title feat: Add spec "/proc/modules" and parser feat: add "modinfo_all" back Jun 21, 2022
@huali027 huali027 changed the title feat: add "modinfo_all" back feat: add "modinfo_all" back and enhance it to get modinfo for all filtered modules Jun 21, 2022
…ltered modules

* Update spec "modinfo" to filterable
* Deprecate all other modinfo specs

Signed-off-by: Huanhuan Li <[email protected]>
@huali027
Copy link
Contributor Author

@xiangce Please help to review, thanks.

Copy link
Contributor

@xiangce xiangce left a comment

Choose a reason for hiding this comment

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

It would be better to explain or give an example of how to use the filter for this spec since the filter does not work with the directly dependent Specs.modinfo_all.

@@ -387,7 +387,7 @@ class Specs(SpecSet):
modinfo_ixgbe = RegistryPoint()
modinfo_veth = RegistryPoint()
modinfo_vmxnet3 = RegistryPoint()
modinfo = RegistryPoint(multi_output=True)
modinfo = RegistryPoint(filterable=True)
Copy link
Contributor

Choose a reason for hiding this comment

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

Since removing the multi_output=True will change the usage of the Parser, better to use another symbolink name.

-----------------------------------------------
for all modules listed by ``lsmod``

ModInfoAll - Command ``modinfo *(filtered modules)``
Copy link
Contributor

Choose a reason for hiding this comment

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

Since there is a All in the name ModInfoAll, it makes me think this is the parser for all the modules loaded on this host. How about using another name? like ModuleInfo

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@xiangce, good to me, I'll update it.

@xiangce
Copy link
Contributor

xiangce commented Jun 27, 2022

@huali027 - please add a short description to the MR in the first comment.

@huali027 huali027 changed the title feat: add "modinfo_all" back and enhance it to get modinfo for all filtered modules feat: add "modinfo_filtered_modules" to collect the filtered modules information Jun 27, 2022
* Add new spec "modinfo_modules" to collect the filtered modules
* Add another new spec "modinfo_filtered_modules" to run "modinfo filtered_modules"
  since the result will be filtered if the spec is filterable, and then other info
  without the filter words will be ignored, which is not expected.

Signed-off-by: Huanhuan Li <[email protected]>
@huali027
Copy link
Contributor Author

@xiangce updated based on the comments, please help to review.

Signed-off-by: Huanhuan Li <[email protected]>
Copy link
Contributor

@xiangce xiangce left a comment

Choose a reason for hiding this comment

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

It looks good to me.

@xiangce xiangce merged commit b134ec5 into RedHatInsights:master Jun 30, 2022
xiangce pushed a commit that referenced this pull request Jun 30, 2022
…information (#3447)

* feat: add "modinfo_all" back and enhance it to get modinfo for all filtered modules

* Update spec "modinfo" to filterable
* Deprecate all other modinfo specs

Signed-off-by: Huanhuan Li <[email protected]>

* feat: Add new specs instead of updating original spec directly

* Add new spec "modinfo_modules" to collect the filtered modules
* Add another new spec "modinfo_filtered_modules" to run "modinfo filtered_modules"
  since the result will be filtered if the spec is filterable, and then other info
  without the filter words will be ignored, which is not expected.

Signed-off-by: Huanhuan Li <[email protected]>

* fixes: Fix failed test

Signed-off-by: Huanhuan Li <[email protected]>
(cherry picked from commit b134ec5)
xiangce pushed a commit that referenced this pull request Sep 6, 2024
…information (#3447)

* feat: add "modinfo_all" back and enhance it to get modinfo for all filtered modules

* Update spec "modinfo" to filterable
* Deprecate all other modinfo specs

Signed-off-by: Huanhuan Li <[email protected]>

* feat: Add new specs instead of updating original spec directly

* Add new spec "modinfo_modules" to collect the filtered modules
* Add another new spec "modinfo_filtered_modules" to run "modinfo filtered_modules"
  since the result will be filtered if the spec is filterable, and then other info
  without the filter words will be ignored, which is not expected.

Signed-off-by: Huanhuan Li <[email protected]>

* fixes: Fix failed test

Signed-off-by: Huanhuan Li <[email protected]>
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