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

Use VM install repo URL on the installed system #4338

Merged
merged 1 commit into from
May 10, 2019

Conversation

comps
Copy link
Collaborator

@comps comps commented May 9, 2019

Fedora doesn't need this (installs mirrors implicitly) and the list can be extended if this logic is checked to work with ie. SLES or openSUSE.

I don't like the gpgcheck=0, but the VM is supposed to be transient and this works with internal or otherwise temporary unsigned repositories.

@pep8speaks
Copy link

pep8speaks commented May 9, 2019

Hello @comps! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2019-05-09 16:09:24 UTC

Signed-off-by: Jiri Jaburek <jjaburek@redhat.com>
@comps
Copy link
Collaborator Author

comps commented May 9, 2019

Fixed the PEP8 issue.

@scrutinizer-notifier
Copy link

The inspection completed: 1 updated code elements

@@ -83,6 +85,25 @@ def parse_args():
return parser.parse_args()


def detect_url_os(url):
"""Given a repository URL, guess an OS type and version."""
cmd = ["osinfo-detect", "-f", "env", "-t", "tree", url]
Copy link
Member

Choose a reason for hiding this comment

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

Please also add libosinfo package into the "Libvirt backend" section of tests/README.md.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@matusmarhefka libosinfo is already used and required by virt-install, so if you have virt-install you already have libosinfo. In addition, the python exception takes care of cases where it's not present and the repository is simply not set up (still allowing some level of testing or manual repo setup).

Are you sure you want me to put it in README.md? .. Maybe as "optional"?

Copy link
Member

Choose a reason for hiding this comment

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

You are right. In that case there is no need to put the package there.

@matusmarhefka matusmarhefka added this to the 0.1.45 milestone May 10, 2019
@matusmarhefka matusmarhefka self-assigned this May 10, 2019
@matusmarhefka
Copy link
Member

@comps Good stuff, thanks for the extension.

@matusmarhefka matusmarhefka merged commit 3021c05 into ComplianceAsCode:master May 10, 2019
@comps comps deleted the add-vm-install-repo branch August 6, 2019 10:16
This pull request was closed.
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.

4 participants