aurora-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Maxim Khutornenko" <ma...@apache.org>
Subject Re: Review Request 31916: Add scheduler API support for audit messages when changing job updates.
Date Wed, 11 Mar 2015 01:20:16 GMT


> On March 11, 2015, 12:19 a.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java, line
48
> > <https://reviews.apache.org/r/31916/diff/1/?file=890852#file890852line48>
> >
> >     Do we really want to fail an operation when a message gets too long? Since it's
optional anyway, I'd expect truncation could be a more resilient way to handle this.
> 
> David McLaughlin wrote:
>     I think it's better to just give a clear message telling them there is a limit. Truncation
could happen in the client if needed.
> 
> Maxim Khutornenko wrote:
>     I was considering a case when some automated external service would attempt to act
on an update and append some arbitrary metadata with pause/resume/abort. While not desirable,
does not necessarily warrant a failure. Stopping a misbehaving update should clearly be a
higher priority than enforcing a strict audit mode.
> 
> Bill Farner wrote:
>     Maybe it does warrant a failure, though.  IMHO truncation would be a policy decision
that the scheduler is making on behalf of the client.  If the most important part of the message
is after the truncation, we've made a poor choice.

IDK, this whole length enforcment seems quite arbitrary to me. We don't restrict string size
anywhere in thrift API or SQL schema. The only exception is TaskIdGenerator where the ID length
is critical for external reasons (MESOS-691). Perhaps we should start doing it everywhere
then?


- Maxim


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


On March 11, 2015, 12:04 a.m., Bill Farner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31916/
> -----------------------------------------------------------
> 
> (Updated March 11, 2015, 12:04 a.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Joshua Cohen.
> 
> 
> Bugs: AURORA-1077
>     https://issues.apache.org/jira/browse/AURORA-1077
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Add scheduler API support for audit messages when changing job updates.
> 
> 
> Diffs
> -----
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift badb8eec51d9034fdfee79061c777927b2ba1314

>   src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java c0d48034ad6b6a91f1f58aca54544a5eddea4742

>   src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java 78024a8c257f2d370a4b4d1ba79c6eac68d81ac2

>   src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java 58824888a4839b71f4efa832a6d62ff6dd946e40

>   src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
7f5e5c2091458192d9310a81314f3c2c42b11f49 
>   src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java eebe01b161fbebdc43e62df4836136a02c3d5fb7

>   src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java e119c494de8e81d7e2dd1f78337f08dcba3cd518

> 
> Diff: https://reviews.apache.org/r/31916/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Bill Farner
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message