WIP: Validate file and services

[funkyfuture]

This is an alternate approach to #1348 in order to solve #129

still failing due to a missing exception, hackish, quiet some TODOs, but it's time for GOT now.

[aanand]

Nice. I think this is a good approach. Looking forward to seeing more.

[funkyfuture]

@aanand @bfirsh @dnephin
alors, i'm at the point where i need a first review while i'm proceeding to work on the validator-library to get some pretty error messages eventually.

i know this is a big commit including various changes that makes it hard to review. feel free to request what can make that easier.

here are some useful resources concerning the used library:
https://cerberus.readthedocs.org
https://github.com/nicolaiarocci/cerberus
which on the way gained some improvements and more is yet to come.

the paradigm of the validation is to be as non-blocking as possible. only errors in a configuration-file as a whole detected by a FileValidator will stop the validation. otherwise any service-dictionary will be checked before aborting.

in order to achieve that i applied two strategies:

• functions that were solely used by ServiceLoader and are directly related to service-creation were made methods of ServiceLoader to access its validator and thus aggregating errors.
• functions that are used from different parts of the code and operate regardless of context do now take an optional validator-instance to report to.

btw, that restructuring makes the diff-view on config.py disturbing to read.

one thing i don't like is that some values can be Service- or Container-instances which requires custom type-validators. which is more code. however i'm not certain whether is only used by test code or could also happen in the real world. in the first case the test code may be refactored.

i haven't yet investigated the failing test. atm it seems strange as the validation code doesn't change values (it even operates with copies).

beside lacking an output of the aggregated errors for now, there are some TODOs and FIXMEs annotated. some with question marks that are a proposal and no necessary change.

one last remark, a coercing-functionality cerberus was recently added, but isn't customizable yet.
some context-less parsing could be be done with it, like splitting mappings to tuples. if so, then certainly as a later commit.

[aanand]

Thanks! I'm going to take a proper look at this later, but:

one thing i don't like is that some values can be Service- or Container-instances which requires custom type-validators. which is more code. however i'm not certain whether is only used by test code or could also happen in the real world. in the first case the test code may be refactored.

It definitely happens in the real world: volumes_from: FOO and net: container:FOO let you specify either a container name or service name as FOO. Arguably that wasn't the best design ever, but we're stuck with it now.

I haven't looked at the new code yet, but as I understand it, the current logic is that we check to see if there's a service called FOO, and if not, assume it's a container name. That means it can still fail at container creation time, but I think that's OK?

[funkyfuture]

I haven't looked at the new code yet, but as I understand it, the current logic is that we check to see if there's a service called FOO, and if not, assume it's a container name. That means it can still fail at container creation time, but I think that's OK?

not sure if there's misunderstanding, it's not about names, but about service- and container-objects (e.g. tested in compose.validators.ServiceValidator._validate_type_container). since the validation code is reached solely through compose.config.load which always loads a file, my assumption is that only names (as strings from a yaml-file) are eventually passed to the validation code.

@GordonTheTurtle nah, my rebase was just fucked up.

[dnephin]

Thanks for the contribution! As you know this was implemented in #1808

[funkyfuture]

just to keep that clear: the scope of this pr was much broader than #1808.

i'll see that i write an elaborated memo on what's missing imo when i have the capacities for it.