Opened 3 years ago

Last modified 3 years ago

#5037 assigned enhancement

Remaining SimpleParser migration work (master ticket)

Reported by: tomek Owned by:
Priority: low Milestone: Outstanding Tasks
Component: remote-management Version: git
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity: N/A
Sub-Project: Mozilla Feature Depending on Ticket:
Estimated Difficulty: 0 Add Hours to Ticket: 0
Total Hours: 0 Internal?: no

Description (last modified by tomek)

This ticket went through a number of evolutions. Initially, it was scoped to cover global DHCPv6 and Subnet6 parsers migration, but that task was mostly done in #5019, so it was rescoped to cover remaining work. However, several child tickets were created for this task, so it was rescoped again as a master ticket that will remain open until all the sub-tasks are complete.

Subtickets

#5096: DhcpX: migrate database configurationclosedfdupont
#5097: DhcpX: Migrate Pools configurationclosedfdupont
#5098: DhcpX: Migrate client class configurationclosedfdupont
#5121: Add HRMode keywords to flex/bisonclosedfdupont
#5122: SubnetParser: migrate initSubnetclosedtomek
#5123: fix SimpleParser exceptionclosedfdupont
#5124: handle required parameters in grammarclosedfdupont
#5125: update SimpleParser templatesclosed
#5126: migrate to SimpleParser "flat style"closedfdupont
#5127: migration cleanupnew
#5128: handle two (or more) occurrences of the same parameteracceptedfdupont
#5147: get rid of bind10 .spec filesnew
#5148: parser/config exception cleanupnew

Change History (31)

comment:1 Changed 3 years ago by tomek

  • Component changed from configuration to remote-management

comment:2 Changed 3 years ago by stephen

  • Milestone changed from Kea1.2 to Kea 1.2 - Mozilla Milestone 1

Moved to milestone "Kea 1.2 - Mozilla Milestone 1" as a result of Kea meeting on 15 December 2016

comment:3 Changed 3 years ago by tomek

  • Owner set to tomek
  • Status changed from new to assigned

comment:4 Changed 3 years ago by tomek

  • Owner tomek deleted

comment:5 Changed 3 years ago by tomek

Add a subticket #5096.

comment:6 Changed 3 years ago by tomek

Add a subticket #5097.

comment:7 Changed 3 years ago by tomek

Add a subticket #5098.

comment:8 Changed 3 years ago by tomek

  • Priority changed from high to medium

That top level parser will be migrated last. It makes more sense, because otherwise we would be updating the code twice - once when the top level parser is migrated, then every time its child parsers are migrated. Hence decreasing priority to regular value (medium).

comment:9 Changed 3 years ago by fdupont

  • Description modified (diff)

comment:10 Changed 3 years ago by wlodekwencel

Please let me know when you merge that ticket :)

comment:11 Changed 3 years ago by fdupont

As the work should be done in #5019 I suggest to keep this ticket for final cleanups:

  • put parsers in the same order in docs, json_config_parser.cc, flex and bison files, etc
  • revisit after parsing stuff as load libraries.
  • uniformize error messages (cap or not, quotes, location, etc)
  • make a plan for more unit tests (we should get coverage results too)
  • easy simple parser improvements

comment:12 follow-up: Changed 3 years ago by tomek

Majority of the work has been done in #5019, but there are couple remaining ones in addition to those that Francis mentioned:

  • once #5097 is merged, migrate remaining part of SubnetXConfigParser: initSubnet still uses getOptionalParam and extracts values from boolean_values_ and string_values_ contexts. Those should be replaced with direct extraction from params using SimpleParser methods.
  • remove SubnetXConfigParser::createSubnetConfigParser

comment:13 Changed 3 years ago by tomek

  • Owner set to tomek

comment:14 in reply to: ↑ 12 Changed 3 years ago by fdupont

Replying to tomek:

Majority of the work has been done in #5019, but there are couple remaining ones in addition to those that Francis mentioned:

  • once #5097 is merged, migrate remaining part of SubnetXConfigParser: initSubnet still uses getOptionalParam and extracts values from boolean_values_ and string_values_ contexts. Those should be replaced with direct extraction from params using SimpleParser methods.
  • remove SubnetXConfigParser::createSubnetConfigParser

=> IMHO we should first fix SimpleParser defects:

  • move DhcpConfigError? (and perhaps ConfigPair?) declarations in a new file in lib/cc which will be included by simpleparser.h and lib/dhcpsrv/parsers/dhcp_config_parser.h
  • raise DhcpConfigError? with location in simple parser tools: the convention should be when DhcpConfigError? is raise there is a location with parenthesis. (i.e. make far easier to track if the location has to be added or not)

Either:

  • add a requireParam tool in simpleparser which must be called before the FOREACH of parsers (the idea is one day the existence of required parameters will be done too in .yy files)
  • with requireParam the getXXX can get the value (vs parent) and be easier to use in FOREACH loop
  • use FOREACH loops in parser to eliminate unknown parameters

Or (as the grammar already eliminates unknown parameters):

  • create a ticket to handle missing required parameters in grammar: more efficient, can put in the error message the syntactic context name with { and } positions (in addition to the location of the keyword aka parent).
  • get rid of FOREACH loops and simply handle each parameter in sequence (aka flat simpleparser style)
  • use defaults when possible so we know parameters exist
  • update my simpleparser templates to use the parent vs value and internally getXXX tools

It seems you prefer the second.

Last edited 3 years ago by fdupont (previous) (diff)

comment:15 Changed 3 years ago by fdupont

Add a subticket #5122.

comment:16 Changed 3 years ago by fdupont

Add a subticket #5123.

comment:17 Changed 3 years ago by fdupont

Add a subticket #5121.

comment:18 Changed 3 years ago by fdupont

Add a subticket #5124.

comment:19 Changed 3 years ago by fdupont

Add a subticket #5125.

comment:20 Changed 3 years ago by fdupont

Add a subticket #5126.

comment:21 Changed 3 years ago by fdupont

Add a subticket #5127.

comment:22 Changed 3 years ago by tomek

  • Description modified (diff)
  • Milestone changed from Kea 1.2 - Mozilla Milestone 1 to Kea1.2
  • Summary changed from Migrate Dhcp6 and Subnet6 to the bison parser to Remaining SimpleParser migration work (master ticket)

Ok, we can't close this ticket, because there are dependencies added. So renaming and moving to 1.2.

comment:23 Changed 3 years ago by fdupont

Add a subticket #5128.

comment:24 Changed 3 years ago by tomek

  • Owner tomek deleted

comment:25 Changed 3 years ago by fdupont

Add a subticket #5147.

comment:26 Changed 3 years ago by fdupont

Add a subticket #5148.

comment:27 Changed 3 years ago by tomek

We will never close this ticket... :)

comment:28 Changed 3 years ago by tomek

  • Sub-Project changed from DHCP to Mozilla

comment:29 Changed 3 years ago by hschempf

  • Priority changed from medium to low

comment:30 Changed 3 years ago by tomek

  • Milestone changed from Kea1.2 to Kea1.2-final

Code freeze for 1.2-beta. Moving all remaining open tickets to 1.2-final.

comment:31 Changed 3 years ago by tomek

  • Milestone changed from Kea1.2-final to Outstanding Tasks

As discussed on 2017-04-13 call, moving to outstanding.

Note: See TracTickets for help on using tickets.