Skip to content

Conversation

fydai
Copy link
Contributor

@fydai fydai commented Feb 9, 2020

Tested both options on scurvy and pandemic

Copy link
Member

@cg505 cg505 left a comment

Choose a reason for hiding this comment

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

thank you!

@@ -0,0 +1 @@
../staff/acct/add-to-group No newline at end of file
Copy link
Member

Choose a reason for hiding this comment

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

should be a separate PR but I don't really care enough

parser.add_argument('--os-type', type=str, default='linux',
help='os type')
parser.add_argument('--os-variant', type=str, default='debian9')
parser.add_argument('--os-variant', type=str, default='debian10')
Copy link
Member

Choose a reason for hiding this comment

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

I thought that debian10 didn't exist in virt-install yet?

Copy link
Member

Choose a reason for hiding this comment

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

osinfo-query os | grep Debian seems to indicate yes on pandemic but not riptide so maybe it's a buster thing?

Copy link
Member

Choose a reason for hiding this comment

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

we probably shouldn't change the default yet in that case.

print('Cancelled.')
sys.exit(2)

if args.ceph and args.vg != 'vg':
Copy link
Member

Choose a reason for hiding this comment

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

we should store the default value in a global variable or something so we only have to change it in one place

Copy link
Member

Choose a reason for hiding this comment

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

use parser.get_default

print('Cancelled.')
sys.exit(2)

if not args.ceph and args.ceph_pool != 'vm':
Copy link
Member

Choose a reason for hiding this comment

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

same as above

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