Rewrite inventory/ini.py #12009

Merged
merged 5 commits into from Aug 20, 2015

Conversation

Projects
None yet
5 participants
@amenonsen
Contributor

amenonsen commented Aug 19, 2015

The new code parses INI-format inventory files in a single pass using a well-documented state machine that reports precise errors and eliminates the duplications and inconsistencies and outright errors in the earlier three-phase parsing code (e.g. three ways to skip comments). It is also much easier now to follow what decisions are being taken on the basis of the parsed data.

This has a 馃憤 from @srvg after review and testing.

cc: @abadger @bcoca @jimi-c

@srgvg

This comment has been minimized.

Show comment
Hide comment
@srgvg

srgvg Aug 19, 2015

Member

馃憤 tested it by dumping my big inventory with lots of ini's into a json, and compared versions with old and new ini.py, which are identical

Member

srgvg commented Aug 19, 2015

馃憤 tested it by dumping my big inventory with lots of ini's into a json, and compared versions with old and new ini.py, which are identical

amenonsen added some commits Aug 14, 2015

Rewrite the INI InventoryParser
The new code parses INI-format inventory files in a single pass using a
well-documented state machine that reports precise errors and eliminates
the duplications and inconsistencies and outright errors in the earlier
three-phase parsing code (e.g. three ways to skip comments). It is also
much easier now to follow what decisions are being taken on the basis of
the parsed data. The comments point out various potential improvements,
particularly in the area of consistent IPv6 handling.

On the ornate marble tombstone of the old code, the following
inscription is one last baffling memento from a bygone age:

-    def _before_comment(self, msg):
-        ''' what's the part of a string before a comment? '''
-        msg = msg.replace("\#","**NOT_A_COMMENT**")
-        msg = msg.split("#")[0]
-        msg = msg.replace("**NOT_A_COMMENT**","#")
-        return msg
Make _parse take an array of input lines as an argument
(There's no compelling reason to do this right now, but should be parser
need to be called multiple times in future, this makes it easier.)
Sanitize IPv6 hostname/port handling
Now we accept IPv6 addresses _with port numbers_ only in the standard
[xxx]:NN notation (though bare IPv6 addresses may be given, as before,
and non-IPv6 addresses may also be placed in square brackets), and any
other host identifiers (IPv4/hostname/host pattern) as before, with an
optional :NN suffix.

abadger added a commit that referenced this pull request Aug 20, 2015

@abadger abadger merged commit ceb66c7 into ansible:devel Aug 20, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@abadger

This comment has been minimized.

Show comment
Hide comment
@abadger

abadger Aug 20, 2015

Member

Thanks for the code cleanup, fixes, and testing guys!

Merged

Hi!

This has been merged in, and will also be included in the next major release.

If you or anyone else has any further questions, please let us know by stopping by one of the two mailing lists, as appropriate:

Because this project is very active, we're unlikely to see comments made on closed tickets, but the mailing list is a great way to ask questions, or post if you don't think this particular
issue is resolved.

Thank you!

Member

abadger commented Aug 20, 2015

Thanks for the code cleanup, fixes, and testing guys!

Merged

Hi!

This has been merged in, and will also be included in the next major release.

If you or anyone else has any further questions, please let us know by stopping by one of the two mailing lists, as appropriate:

Because this project is very active, we're unlikely to see comments made on closed tickets, but the mailing list is a great way to ask questions, or post if you don't think this particular
issue is resolved.

Thank you!

@amenonsen amenonsen deleted the amenonsen:inventory-ini-rewrite branch Aug 31, 2015

@ansibot ansibot added feature and removed feature_pull_request labels Mar 4, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment