Repository navigation
MarathonGroup().from_json() does not work for root group ("/") #227
Description
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,
Activity
solarkennedy commentedon Oct 26, 2017
Yes.
daltonmatos commentedon Oct 26, 2017
Great! I will submit the Pull request.
I just found out that this same validation breaks
MarathonClient().get_group("/").