aurora-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Santhosh Kumar Shanmugham <santhoshkuma...@gmail.com>
Subject Re: Review Request 51384: Introduce UpdateMetadata fields in JobUpdateRequest.
Date Tue, 30 Aug 2016 04:38:43 GMT


> On Aug. 29, 2016, 11:52 a.m., Joshua Cohen wrote:
> > src/main/python/apache/aurora/client/cli/update.py, line 193
> > <https://reviews.apache.org/r/51384/diff/4/?file=1487511#file1487511line193>
> >
> >     Am I missing something? Shouldn't this be a set of `Metadata` structs? rather
than a set of strings which is what this will currently pass?
> 
> Santhosh Kumar Shanmugham wrote:
>     You are totally correct. Is there any way to make the python tests do type-checking?
>     
>     This however makes the "assert mock_calls" on mock_api to fail. python's assert,
TestCase.assertEquals and testfixtures.compare all are unable to deal with Metadata object.
Any ideas around this would be welcome.
> 
> Joshua Cohen wrote:
>     I think what I would recommend is updating `_job_update_request` to take a dict of
metadata key/value pairs and performing the thrift conversion there. That has the benefit
of encapsulating knowledge of the thrift API within the API client. It also means you should
be able to assert on the dict that's passed in test_supdate?
>     
>     That said, simple equality on the generated thrift types should work? E.g.
>     
>         $ PEX_INTERPRETER=1 aurora
>         >>> from gen.apache.aurora.api.ttypes import Metadata
>         >>> a = Metadata('foo', 'bar')
>         >>> b = Metadata('foo', 'bar')
>         >>> a == b
>         True
>         >>> c = Metadata('baz', 'qux')
>         >>> a == c
>         False

Items of a set are not getting deep equality check. Is there an assertDeepEquals() ?

$ PEX_INTERPRETER=1 aurora
>>> from gen.apache.aurora.api.ttypes import Metadata
>>> a = Metadata('foo', 'bar')
>>> b = Metadata('foo', 'bar')
>>> a == b
True
>>> {a} == {b}
False
>>> set([a]) == set([b])
False
>>> set([a]) == set([a])
True


- Santhosh Kumar


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


On Aug. 27, 2016, 8:58 p.m., Santhosh Kumar Shanmugham wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51384/
> -----------------------------------------------------------
> 
> (Updated Aug. 27, 2016, 8:58 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Joshua Cohen, and Maxim Khutornenko.
> 
> 
> Bugs: AURORA-1711
>     https://issues.apache.org/jira/browse/AURORA-1711
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Allow custom metadata to be stored with the job updates. This feature
> is to help case the reconciliation on job update request on the clients.
> 
> Today when a client's requests are proxied via a middle-man,
> if the middle-man times out before the scheduler responds with success,
> the client is left the only option of retry. In the case of updates,
> since multiple updates are not allowed in parallel, the retries fail,
> with INVALID_REQUEST. Although the original update is already accepted
> by the scheduler and is in-progress.
> 
> With this feature, clients can reconcile the job updates,
> - by marking updates them with a unique id in the metadata field
> - scheduler returns the in-progress job update in its response
> - client can match the client-generated ids to reconcile its state.
> 
> 
> Diffs
> -----
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift c5765b70501c101f0535b4eed94e9948c36808f9

>   src/main/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStore.java d2673e6b328cb1e249fbe91d18e0d9e935636eaa

>   src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java a3b04949f1726f110638e4f93ef45947cdb9e7f8

>   src/main/java/org/apache/aurora/scheduler/storage/db/migration/V008_CreateUpdateMetadataTable.java
PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/storage/db/views/DbJobUpdate.java 78703e92c932cd5e93ff0b70f2a0b6928f6b4003

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

>   src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java 594bb6219294dcc77d48dcad14e2a6f9caa0c534

>   src/main/java/org/apache/aurora/scheduler/updater/UpdateInProgressException.java PRE-CREATION

>   src/main/python/apache/aurora/client/api/__init__.py 9149c3018ae58d405f284fcbd4076d251ccc8192

>   src/main/python/apache/aurora/client/cli/context.py f1a256a8d09d23d8d4d4ee7d264be0fe376398c4

>   src/main/python/apache/aurora/client/cli/options.py 1245ff15a69a4b4347672f7b556985521e813a00

>   src/main/python/apache/aurora/client/cli/update.py 23aaa2c1b67599420408633733e4581553f7151b

>   src/main/python/apache/aurora/client/hooks/hooked_api.py 542f165add0f1d01a782fe6d6089bff3e736fb82

>   src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
8563630a179cb6d1e3b0e22c730ccbfe1c9291e2 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql a40830fd668aa650c22a48cbc757b45aef27e961

>   src/test/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStoreTest.java 08530397ff75081bde6f07f9d53317b5486e0da4

>   src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
779dc302602ae8842084807ca868a491ea99b676 
>   src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 04551f17999d742c53dfb1b36286b119b448550e

>   src/test/python/apache/aurora/client/cli/test_supdate.py 92fb039fb7d3e368d7c0dfa5ebdb465c7f112416

>   src/test/python/apache/aurora/client/cli/util.py aac9f9c802e4ad1f06cee8cf3f56749760b33af5

> 
> Diff: https://reviews.apache.org/r/51384/diff/
> 
> 
> Testing
> -------
> 
> ./build-support/jenkins/build.sh
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> ./pants test src/test/python/apache/aurora/client/cli:cli
> 
> 
> Thanks,
> 
> Santhosh Kumar Shanmugham
> 
>


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