Linaro Image Tools

Merge lp://qastaging/~milo/linaro-image-tools/hwpack-format-converter into lp://qastaging/linaro-image-tools/11.11

hwpack-format-converter
Merge into trunk

Proposed by Milo Casagrande on 2012-07-19

Status:	Merged
Merged at revision:	535
Proposed branch:	lp://qastaging/~milo/linaro-image-tools/hwpack-format-converter
Merge into:	lp://qastaging/linaro-image-tools/11.11
Diff against target:	657 lines (+617/-0) 6 files modified linaro-hwpack-convert (+73/-0) linaro_image_tools/hwpack/hwpack_convert.py (+297/-0) linaro_image_tools/hwpack/hwpack_fields.py (+88/-0) linaro_image_tools/hwpack/tests/__init__.py (+1/-0) linaro_image_tools/hwpack/tests/test_hwpack_converter.py (+132/-0) linaro_image_tools/tests/fixtures.py (+26/-0)
To merge this branch:	bzr merge lp://qastaging/~milo/linaro-image-tools/hwpack-format-converter
Related bugs:	Link a bug report

Reviewer	Date Requested	Status
Данило Шеган (community)	2012-07-19	Approve on 2012-07-19
Stevan Radaković	2012-07-19	Pending
Linaro Infrastructure	2012-07-19	Pending
Review via email: mp+115658@code.qastaging.launchpad.net

Description of the change

Finally, a first merge.

I split it out into a smaller one, othere will follow. In this one there is only the hwpack format converter from INI style into YAML.

The script is called linaro-hwpack-convert and only one argument is necessary: the input file. If no output name is given, it will write in the same place of the input file, with the same name plus the '.yaml' suffix.

All the conversion logic is in the hwpack_converter.py file, a class called HwpackConverter. In the same file there are a series of methods that are used for the conversion itself, and that will be used also in other parts of l-i-t in order to create the new metadata file.

I added also another file, hwpack_fields, that holds all the fields that should be in a hwpack configuration file.
For the test part, tests are provided for the converter: I added a new mocking feature to create a temporary file (I had problems using StrinIO() in this case).

Revision history for this message

Milo Casagrande (milo) wrote on 2012-07-19:

Another note.

Starting from this INI file:
https://pastebin.linaro.org/691/

You will get this YAML file:
https://pastebin.linaro.org/688/

That INI file can be found here:
http://bazaar.launchpad.net/~linaro-maintainers/linaro-images/hwpack.precise.linaro-lt-panda/view/head:/hwpacks/linaro-lt-panda

Revision history for this message

Данило Шеган (danilo) wrote on 2012-07-19:

As discussed on IRC, let's try out using

yaml.dump(..., default_flow_style=False)

It should cut down on the code size significantly.

review: Needs Fixing

lp://qastaging/~milo/linaro-image-tools/hwpack-format-converter updated on 2012-07-19

536. By Milo Casagrande on 2012-07-19: Used yaml.dump, cleaned up code and tests.

Revision history for this message

Milo Casagrande (milo) wrote on 2012-07-19:

With the latest push this is now done.

--
Milo Casagrande
Infrastructure Engineer
Linaro.org <www.linaro.org> │ Open source software for ARM SoCs

Revision history for this message

Данило Шеган (danilo) wrote on 2012-07-19:

New files are missing the license header.

It would be nice if get_logger (which exists in similar form in a few other files; though not named like that) was shared throughout, but it's not a requirement for this branch.

Also, why did we not reuse the code from hwpack/config.py that already does all of the parsing of the existing config files? I understand we do need some special handling, but not all of it needs it.

Tests have a few alignment problems, eg:

460 + self.assertRaises(HwpackConverterException, check_and_validate_args,
461 + Args(input_file=temp_file.name,
462 + output_file=temp_dir))

This should be:

460 + self.assertRaises(HwpackConverterException, check_and_validate_args,
461 + Args(input_file=temp_file.name,
462 + output_file=temp_dir))

or, even better:

460 + self.assertRaises(
461 + HwpackConverterException, check_and_validate_args,
462 + Args(input_file=temp_file.name, output_file=temp_dir))

With the small fixes for things that are mentioned (no need to switch over to the existing Config parser), this looks good to land.

review: Approve

Revision history for this message

Данило Шеган (danilo) wrote on 2012-07-19:

ok, that didn't come out too well :) https://pastebin.linaro.org/694/

Revision history for this message

Milo Casagrande (milo) wrote on 2012-07-19:

On Thu, Jul 19, 2012 at 1:54 PM, Данило Шеган <email address hidden> wrote:
>
> New files are missing the license header.
>
> It would be nice if get_logger (which exists in similar form in a few other files; though not named like that) was shared throughout, but it's not a requirement for this branch.

I can do that with the coming up merges, it shouldn't take much. Just
need to know which one to use.

> Also, why did we not reuse the code from hwpack/config.py that already does all of the parsing of the existing config files? I understand we do need some special handling, but not all of it needs it.

I though about using the Config, but then I would have had to go
through a long series of "if config.$PROPERTY is not None" (as we
already do in the metadata), but what we really need to pay attention
to and treat differently are just those values, not all of the Config
possible ones.
Also, if we get rid of V2 handling in the code, we might want to keep
the converter around if somebody still has those configs.

Switching is not that hard, indeed, just a little bit of if-clauses to write.

> Tests have a few alignment problems, eg:

These are fixed.

--
Milo Casagrande
Infrastructure Engineer
Linaro.org <www.linaro.org> │ Open source software for ARM SoCs

lp://qastaging/~milo/linaro-image-tools/hwpack-format-converter updated on 2012-07-19

537. By Milo Casagrande on 2012-07-19: Added license header, fixed boolean values.
538. By Milo Casagrande on 2012-07-19: Fixed indentation, remove unused import.