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: Parser for "ls systemd units" #3451

Merged
merged 5 commits into from
Jun 30, 2022

Conversation

jobselko
Copy link
Contributor

@jobselko jobselko commented Jun 23, 2022

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?

Complete Description of Additions/Changes:

This PR aims to consolidate all "ls systemd units" parsers into one file. The combiner will be created afterwards.

MR with relevant changes in insights-core-assets: !437

@jobselko jobselko requested review from xiangce and bfahr June 23, 2022 11:59
@xiangce
Copy link
Contributor

xiangce commented Jun 24, 2022

@jobselko - Would you like to merge all these ls systems units specs into one? just the ls_etc:

etc_and_sub_dirs = sorted(["/etc", "/etc/pki/tls/private", "/etc/pki/tls/certs",
"/etc/pki/ovirt-vmconsole", "/etc/nova/migration", "/etc/sysconfig",
"/etc/cloud/cloud.cfg.d", "/etc/rc.d/init.d"])
ls_etc = simple_command("/bin/ls -lan {0}".format(' '.join(etc_and_sub_dirs)), keep_rc=True)

e.g.

 systemd_dirs = sorted(["/etc/systemd", "/run/systemd", "/usr/lib/systemd", "/usr/local/lib/systemd"]) 
 ls_systemd_utils = simple_command("/bin/ls -lanRL {0}".format(' '.join(systemd_dirs)), keep_rc=True)

In this way, all the ls results will be wrapped into one simple_file and the combiner you mentioned may be not required.

@psachin
Copy link
Contributor

psachin commented Jun 24, 2022

Hey @jobselko I think we should deprecate the parsers instead of deleting them directly since this is a public repo we don't know whether the parser is already in use /outside Red Hat/. A deprecating warning will alert the user against the alternative parser or the new module where it is migrated (in this case)

Signed-off-by: Jitka Obselkova <[email protected]>
@jobselko
Copy link
Contributor Author

jobselko commented Jun 28, 2022

@xiangce I reworked it. Thank you!
@psachin I do not think this is a problem. These parsers are unused (by our teams) and were added around three months ago.

@jobselko jobselko changed the title Parsers for "ls systemd units" Parser for "ls systemd units" Jun 29, 2022
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.

@jobselko it looks good to me

@jobselko
Copy link
Contributor Author

I came across these bugs: #3453, #3454

@xiangce
Copy link
Contributor

xiangce commented Jun 30, 2022

@jobselko - You missed updating the insights_archive.py file in this PR

Signed-off-by: Jitka Obselkova <[email protected]>
@xiangce
Copy link
Contributor

xiangce commented Jun 30, 2022

The python26-test in CI failed due to an environmental issue.

Certificate did not match expected hostname: pypi.python.org.

I'll ignore it and merge this PR

@xiangce xiangce changed the title Parser for "ls systemd units" feat: Parser for "ls systemd units" Jun 30, 2022
@xiangce xiangce merged commit bc6fff8 into RedHatInsights:master Jun 30, 2022
xiangce pushed a commit that referenced this pull request Jun 30, 2022
* Create a new file for ls systemd units parsers

Signed-off-by: Jitka Obselkova <[email protected]>

* Remove separate files for ls systemd units parsers

Signed-off-by: Jitka Obselkova <[email protected]>

* Remove unused specs

Signed-off-by: Jitka Obselkova <[email protected]>

* Rework parsers into one

Signed-off-by: Jitka Obselkova <[email protected]>

* Update insights_archive

Signed-off-by: Jitka Obselkova <[email protected]>
(cherry picked from commit bc6fff8)
xiangce pushed a commit that referenced this pull request Sep 6, 2024
* Create a new file for ls systemd units parsers

Signed-off-by: Jitka Obselkova <[email protected]>

* Remove separate files for ls systemd units parsers

Signed-off-by: Jitka Obselkova <[email protected]>

* Remove unused specs

Signed-off-by: Jitka Obselkova <[email protected]>

* Rework parsers into one

Signed-off-by: Jitka Obselkova <[email protected]>

* Update insights_archive

Signed-off-by: Jitka Obselkova <[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