> On Aug. 29, 2016, 6:52 p.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
> 
> Santhosh Kumar Shanmugham wrote:
>     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

Ahh, the thrift python compiler did not generate `__hash__` methods until this 
commit: 
https://github.com/apache/thrift/commit/e841b3dac619a5e5d3523d059d48db1a12e41360,
 which does not seem to have made it into an official release yet (not that it 
would matter if it did, since we're pinned to an older version of thrift for 
reasons I cannot recall at the moment).

So I think you'd have to iterate the sets to check for equality (which would be 
made more complicated by the fact that ordering is not guaranteed). All the 
more reason to just bury the set of thrift structs internal to the API impl and 
just pass a dict instead?


- Joshua


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


On Aug. 28, 2016, 3:58 a.m., Santhosh Kumar Shanmugham wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51384/
> -----------------------------------------------------------
> 
> (Updated Aug. 28, 2016, 3:58 a.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
> 
>

Reply via email to