Re: [wildfly-dev] Management Parser Versioning

The following commits are now the end result of the re-factoring to allow a fork on new major versions and also to add version 4.0 of the schema: - https://github.com/wildfly/wildfly-core/compare/f4a07f4e9b41dc59823fea181... The changes look extensive but TBH the bulk is just moving of existing code and some duplicate for the fork for 4. I have distinct commits to show the steps taken but the key points are: - # Split out the model specific parsing from CommonXml into smaller implementations, this was to allow CommonXml to remain the common base without still containing version specific parsing. # Rework of ManagementXml to be version based and rework the delegate, the big problem we had was we were looping back on ourselves with static calls which did not cleanly map to versioned implementations. # Moved existing StandalonXml, HostXml, DomainXml, ManagementXml into 'Legacy' parsers and used existing class to load these on-demand. # Forked the legacy parsers to create the version 4 parsers, removed all version handling so a clean start for 4. At this point there is still plenty of parsing code not forked for 4 but TBH those are all relatively stable and rarely change, StandaloneXml, HostXml, DomainXml, and ManagementXml on the other hand have changed for every major version so far. Regards, Darran Lofthouse. On 23/04/15 16:37, Darran Lofthouse wrote: > On 23/04/15 16:32, Brian Stansberry wrote: >> I don't think I've ever rejected or even questioned a PR because it made >> a parser tolerant in the way you describe, so I'm fine with being tolerant. > > +1 I think the first two rules are what we need to advertise we support, > after all that is why we have schemas. The third point is more about > the requirements being placed on the maintainer of the parser so if your > XML does not match the schema the best we say is "It may work, it may > not work and that can change without warning". > >> My only caveat is in no way do we have any commitment to be tolerant. If >> a change is implemented in such a way that the legacy schema parsing is >> tolerant, then it's tolerant for that change. Some other change may not >> be implemented that way and the parsing won't be tolerant. So we'll be >> inconsistent. >> >> I'm sure we're already inconsistent in this regard, so that doesn't >> worry me. The schema defines the rules. If you follow them, you know >> what you'll get. If you break them, maybe it will work anyway, maybe >> not. Our only guarantee if you break the rules is you'll either end up >> with a valid running configuration or we'll fail. >> >> On 4/23/15 10:17 AM, Darran Lofthouse wrote: >>> Thinking about future XML changes, once the parser is forked I think we >>> can also relax how we add new optional attributes and elements to the >>> schema. >>> >>> As I see it our main commitment is: - >>> >>> * XML that is valid according to the schema should parse without error. * >>> >>> I say 'should' as I believe in some cases documentation is relied upon >>> to describe valid combinations. >>> >>> * The XML we write must be valid according to the schema. * >>> >>> Don't see any justification for not following that one. >>> >>> * Parsers can be tolerant to XML that is technically invalid. * >>> >>> e.g. we are not big on enforcing sequence ordering. >>> >>> So lets say we have a release that contains a 4.0 version of the schema >>> I think if in version 4.1 of the schema we add an optional attribute or >>> element we can just add support for that attribute or element to the >>> existing parse method. >>> >>> Of course if the new attribute or element is required then we will need >>> to switch to something version specific to avoid rejecting previously >>> valid configuration. >>> >>> I know this is a long way off but I only raise this now as I realise >>> this has already happened where a new optional element has been added to >>> the schema. Technically if we were strict about it the code is wrong >>> but the scenarios where it is going to break for someone are quite >>> contrived and I think this fits with tolerant parsing and we will >>> automatically fix on the next write. >>> >>> Regards, >>> Darran Lofthouse. >>> >>> >>> On 22/04/15 13:30, Brian Stansberry wrote: >>>> That's fine with me. >>>> >>>> On 4/22/15 6:20 AM, Darran Lofthouse wrote: >>>>> Working with the parsers for the core config has become increasingly >>>>> cryptic, we are now at the point where we have three different major >>>>> versions which diverge and converge as we work on them. Most recent >>>>> changes have resulted in large sections of the config converging for 1.x >>>>> and 3.x leaving 2.x independent. >>>>> >>>>> So that I can add references to Elytron I am starting to add support for >>>>> version 4. >>>>> >>>>> One think that I have learned is that each major version tends to belong >>>>> to one branch of the codebase, all changes to that version happen on >>>>> that branch first: - >>>>> >>>>> 1.x - Maintained only for EAP >>>>> 2.x - WildFly 8.x branch >>>>> 3.x - WildFly Core master branch >>>>> >>>>> I would expect if further changes are made to core for WildFly 9 >>>>> releases we will end up with 1.x branch of core and and 4.x version of >>>>> the schema will be owned by the master branch. >>>>> >>>>> To make things less cryptic I am proposing that until we find a better >>>>> solution for all subsequent major schema versions we just fork the >>>>> parser and all related classes. >>>>> >>>>> This will simplify the code being modified for the upstream development. >>>>> >>>>> Forward porting parsing changes will also become a simple copy and paste. >>>>> >>>>> For the current cryptic approach I think almost every engineer (and I am >>>>> finding it really hard to think of exceptions) that has worked in-depth >>>>> in this area has introduced at least one bug and I don't think the test >>>>> coverage is high enough to protect against this. >>>>> >>>>> Regards, >>>>> Darran Lofthouse. >>>>> >>>>> _______________________________________________ >>>>> wildfly-dev mailing list >>>>> wildfly-dev(a)lists.jboss.org >>>>> https://lists.jboss.org/mailman/listinfo/wildfly-dev >>>>> >>>> >>>> >>> _______________________________________________ >>> wildfly-dev mailing list >>> wildfly-dev(a)lists.jboss.org >>> https://lists.jboss.org/mailman/listinfo/wildfly-dev >>> >> >> > _______________________________________________ > wildfly-dev mailing list > wildfly-dev(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/wildfly-dev > _______________________________________________ wildfly-dev mailing list wildfly-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/wildfly-dev