> On March 28, 2018, 2:36 a.m., James DeFelice wrote:
> > include/mesos/agent/agent.proto
> > Line 327 (original), 331 (patched)
> > <https://reviews.apache.org/r/66318/diff/1/?file=1989161#file1989161line331>
> >
> >     "Note that only..."
> >     
> >     I don't think this statement is true. I have e2e tests that add a 
> > provider and then update it. The provider DID NOT exist at agent startup.

You are right. I described the statement in a wrong way. It should be:
"Note that if a config file is placed into the resource provider config 
directory out-of-band after the starts up, it will not be checked against this 
call."


> On March 28, 2018, 2:36 a.m., James DeFelice wrote:
> > include/mesos/agent/agent.proto
> > Line 332 (original), 338 (patched)
> > <https://reviews.apache.org/r/66318/diff/1/?file=1989161#file1989161line338>
> >
> >     Maybe return 409 instead of 404 here, if the caller tries to update a 
> > config that does not exist? Because, you know, 404s can be misleading given 
> > the convo we had earlier...

Yeah I was aware of this issue and discussed with Jie but we still thought 404 
is semantically more correct here. Will probably raise a general question 
related to this on the next API WG.


- Chun-Hung


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66318/#review200089
-----------------------------------------------------------


On March 27, 2018, 11:27 p.m., Chun-Hung Hsiao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66318/
> -----------------------------------------------------------
> 
> (Updated March 27, 2018, 11:27 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, James DeFelice, and Jie Yu.
> 
> 
> Bugs: MESOS-8742
>     https://issues.apache.org/jira/browse/MESOS-8742
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch adds descriptions to declare the following agent API calls
> idempotent:
>   - `ADD_RESOURCE_PROVIDER_CONFIG`
>   - `UPDATE_RESOURCE_PROVIDER_CONFIG`
>   - `REMOVE_RESOURCE_PROVIDER_CONFIG`
> 
> 
> Diffs
> -----
> 
>   include/mesos/agent/agent.proto adaccb3509fdfc1e7cac9482e93a91e83bc5625d 
>   include/mesos/v1/agent/agent.proto 71352a79fe7d28d633a4badceafe18086c1e5ab7 
> 
> 
> Diff: https://reviews.apache.org/r/66318/diff/1/
> 
> 
> Testing
> -------
> 
> N/A
> 
> The actual implementation will be in the next patch.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Reply via email to