-
Notifications
You must be signed in to change notification settings - Fork 1
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
Accept configdrive info as {path: base64_content} style #23
Conversation
filename = os.path.join(location, path) | ||
with open(filename, 'w') as f: | ||
json_data = contents.decode('base64') | ||
f.write(json_data) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not necessarily json data anymore, but :)
Yeah, I think this is fine, +1 |
@pquerna thoughts on this? We had some back-and-forth last week, and I saw you mention this on IRC over the weekend.
I hate base64 as an API as much as the next person, but given that this isn't user-facing, and that we're going to be taking some base64 blobs, I'm alright with this. |
f.write(json_metadata) | ||
|
||
|
||
def write_configdrive(location, metadata, files, prefix='teeth', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this function actually used outside of tests?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this prefix
(and maybe the one above) should default to openstack
. I don't see writing to a teeth
prefix being practical in the near future.
Other than the "teeth" prefix, LGTM. |
Discussed with Chris in IRC this morning and decided this was the best way to go.
This also hardcodes the location on disk to write the configdrive to. I don't think that needs to be parameterized.