-
Notifications
You must be signed in to change notification settings - Fork 909
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
Fix vendordata arrays and dicts #837
Conversation
26bbf36
to
c48c888
Compare
@eb3095 It definitely feels like you found a bug here, that was introduced by #777 (3cebe0d). But I'm not sure that this is the right fix. @andrewbogott, @TheRealFalcon perhaps you have more context? Openstack datasource gets vendordata and vendordata2 from read_v2. If I'm reading that correctly, that will return the loaded json object, so it could be any json object (dict, list, string ... ). Then it calls convert_vendordata on that. convert_vendordata says clearly that it will return a string or list. End result is that @eb3095 is correct, and that store_rawdata has to take any type, not just bytes. It feels like I must be reading this wrong though... because otherwise I don't see how it would have passed any actual testing. Help? |
@smoser I don't think this is a new issue. I can go back a few years and set
|
yeah, you're right. I'm really not sure how this worked at all. this is really a trainwreck. 'b' is much easier. So what I'd suggest is handling vendordata (and vendordata2) specially here and adding 'store_raw_vendordata' method that accepts those as input and serializes a list with json. |
e868022
to
3f1b045
Compare
Is this more in line with what you were looking for here? |
Refactor to request Syntax error Syntax error Syntax error
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.
thank you.
One comment.
06ce89a
to
bf8b8a7
Compare
@eb3095 could you write a single coherent commit message? You can either put it here (in the issue description) or squash your commits and write a single good message and then force-push. Here is a suggested commit message:
We want the commit message to have less than 74 chars per line. Thanks for your patience. |
Looks like this was merged, did you still need that? |
Sorry, no. I merged with the suggested commit message. |
Fix vendordata arrays and dicts
Fix a bug with vendordata_raw when arrays or arrays of dicts are used
Additional Context
@smoser had given me two examples of usage for vendordata_raw for use in the bellow listed PR. However when attempting to use it I encountered an unexpected error regarding writing it to a file. The methods he provided worked otherwise leading me to believe this may be a bug.
#827 (comment)
Test Steps
This is the code I used to trigger this failure. It originates from the following PR.
#827
Checklist: