MarathonGroup().from_json() does not work for root group ("/")

[daltonmatos]

Hello,

Currently it's not possible to construct the root group with MarathonGroup(), the validation in assert_valid_id() fails:

In [1]:         data = {
   ...:             "id": "/",
   ...:             "groups": [
   ...:                 {"id": "/foo", "apps": []},
   ...:                 {"id": "/bla", "apps": []},
   ...:             ],
   ...:             "apps": []
   ...:         }
   ...: 

In [2]: data
Out[2]: 
{'apps': [],
 'groups': [{'apps': [], 'id': '/foo'}, {'apps': [], 'id': '/bla'}],
 'id': '/'}

In [3]: from marathon.models.group import MarathonGroup

In [4]: _g = MarathonGroup().from_json(data)
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
<ipython-input-4-84b8be244b3a> in <module>()
----> 1 _g = MarathonGroup().from_json(data)

marathon-python/marathon/models/base.py in from_json(cls, attributes)
     41         :param dict attributes: object attributes from parsed response
     42         """
---> 43         return cls(**{to_snake_case(k): v for k, v in attributes.items()})
     44 
     45     def to_json(self, minimal=True):

marathon-python/marathon/models/group.py in __init__(self, apps, dependencies, groups, id, pods, version)
     37         #     for p in (pods or [])
     38         # ]
---> 39         self.id = assert_valid_id(id)
     40         self.version = version

marathon-python/marathon/models/base.py in assert_valid_id(id)
    117     if not ID_PATTERN.match(id.strip('/')):
    118         raise ValueError(
--> 119             'invalid id (allowed: lowercase letters, digits, hyphen, ".", ".."): %r' % id)
    120     return id

ValueError: invalid id (allowed: lowercase letters, digits, hyphen, ".", ".."): '/'

The problem is that with this validation, being the way it is now, it's impossible to reconstruct the root group, for example, get the output of /v2/groups/ and pass it to MarathonGroup().from_json().

Do you think it makes sense not to validate the id and let the developer take care of providing valid data?

Thinking about validations, currently there's no validation on the structure of groups and subgroups, so it's already possible to create an invalid group structure, like this:

In [5]: invalid_group_data = {"id": "/teste", "groups": [{"id": "/bla"}, {"id": "/foo"}]}

In [6]: _g = MarathonGroup().from_json(invalid_group_data)

In [7]: _g.id
Out[7]: '/teste'

In [8]: _g.groups
Out[8]: [MarathonGroup::/bla, MarathonGroup::/foo]

In [9]: 

This is an invalid group definition and marathon rejects it:

curl -X POST -d '{"id": "/teste", "groups": [{"id": "/bla"}, {"id": "/foo"}]}' 127.0.0.1:8080/v2/groups | json_pp
{
   "details" : [
      {
         "errors" : [
            "Identifier is not child of /teste. Hint: use relative paths."
         ],
         "path" : "/groups(3)/groups(0)/id"
      },
      {
         "path" : "/groups(3)/groups(1)/id",
         "errors" : [
            "Identifier is not child of /teste. Hint: use relative paths."
         ]
      }
   ],
   "message" : "Object is not valid"
}
      

Using relative paths, it works, as suggested by marathon itself.

curl -X POST -d '{"id": "/teste", "groups": [{"id": "bla"}, {"id": "foo"}]}' 10.168.200.97:8080/v2/groups  
{"version":"2017-10-26T09:37:16.520Z","deploymentId":"83c40525-896f-451e-86a3-cff4c6570dc6"}

What do you think? Would you be OK about removing this validation?

Thanks,

[solarkennedy]

Yes.

[daltonmatos]

Great! I will submit the Pull request.

I just found out that this same validation breaks MarathonClient().get_group("/").