mesosphere / marathon

Deploy and manage containers (including Docker) on top of Apache Mesos at scale.
https://mesosphere.github.io/marathon/
Apache License 2.0
4.07k stars 845 forks source link

Bad JSON prints java types rather than human error message #1331

Closed cmaloney closed 9 years ago

cmaloney commented 9 years ago
{
  "uris:": ["file:///etc/.dockercfg"]
}

when submitted to marathon makes the error message:

{"message":"Invalid JSON: JsResultException(errors:List((,List(ValidationError(error.expected.jsarray,WrappedArray())))))"}
cmaloney commented 9 years ago

A couple more bits about this:

It needs to be HTTP PUT to /v2/apps, the error is coming about because the marathon.json should be an array of items to add apparently (Not obvious to me in porting code from the old HTTP POST api), and isn't immediately obvious reading the changelog that PUT /v2/apps expects a different format than POST /v2/apps

PUT /v2/apps also isn't documented at http://mesosphere.github.io/marathon/docs/rest-api.html#put-/v2/apps/%7Bappid%7D

drexin commented 9 years ago

Why don't you use /v2/apps/{appId}?

cmaloney commented 9 years ago

That would require my bash script to parse the json to figure out the appId then append that to the url...

Complicates interaction significantly. I used to just need one script which did a POST (both for creation and update) to /v2/apps, but that was removed with 0.8.1.

drexin commented 9 years ago

It has been changed because it was wrong behaviour. Trying to create an object with an existing id should be rejected. The PUT endpoint will work for both, creating and updating apps. You could use jq or something similar to extract the appId.

drexin commented 9 years ago

Of course the error message could be better.

cmaloney commented 9 years ago

Forcing extracting the appId considerably complicates every program which wants to use the Marathon API, and practically it is not something structurally or code-wise hard to do. There is actually more code required to make the user specify both, because internally marathon needs to verify the appIds match.

You could say the appId lives outside of the marathon.json, but then marathon.json isn't self-contained as a container launch / specification format. There are some applications where that definitely makes sense (Launching multiple marathons via marathon, one for each user). Others where I'm launching one non0instanced app and just need to update it's config from time to time, definitely don't fit.