Mailing-List: contact reviews-help@aurora.incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: reviews@aurora.incubator.apache.org
Content-Type: multipart/alternative;
 boundary="===============4039062330752257677=="
MIME-Version: 1.0
Subject: Re: Review Request 32313: Rename 'update status' to 'update info' and
 support fetching arbitrary updates.
From: "Zameer Manji" <zmanji@apache.org>
To: "Joshua Cohen" <jcohen@apache.org>, "Zameer Manji" <zmanji@apache.org>
Cc: "Bill Farner" <wfarner@apache.org>,
 "Aurora" <reviews@aurora.incubator.apache.org>
Date: Fri, 20 Mar 2015 17:59:41 -0000
Message-ID: <20150320175941.1490.57731@reviews.apache.org>
Auto-Submitted: auto-generated
Sender: "Zameer Manji" <noreply@reviews.apache.org>
References: <20150320172641.1489.15208@reviews.apache.org>
In-Reply-To: <20150320172641.1489.15208@reviews.apache.org>
Reply-To: "Zameer Manji" <zmanji@apache.org>

--===============4039062330752257677==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit


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

Ship it!


src/test/python/apache/aurora/client/cli/test_supdate.py
<https://reviews.apache.org/r/32313/#comment125117>

    Using a raw mock here is a little bit dangerous if the shape of the raw config changes underneath the test. Would you mind setting the spec argument of the Mock class here? http://www.voidspace.org.uk/python/mock/mock.html#mock.Mock
    
    I was thinking of something like Mock(spec=['has_cron_schedule']). Alternatively you could set the spec to be the Job schema object. I think it could be of the form of Mock(spec=aurora.config.schema.base.Job) or Mock(spec=aurora.config.schema.base.Job()). This might be impossible because Job() is a pystachio object but I strongly suggest investigating it.
    
    This way if we try to access other attributes the test will fail.


- Zameer Manji


On March 20, 2015, 10:26 a.m., Bill Farner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32313/
> -----------------------------------------------------------
> 
> (Updated March 20, 2015, 10:26 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Zameer Manji.
> 
> 
> Bugs: AURORA-1206
>     https://issues.apache.org/jira/browse/AURORA-1206
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> One change i snuck in here is in `cli/__init__.py`.  This makes the subcommand help include the the description:
> 
> ```
> $ aurora update info -h
> usage: aurora update info [-h] [--write-json] [--verbose]
>                           [--skip-hooks hook,hook,...]
>                           CLUSTER/ROLE/ENV/NAME [ID]
> 
> Display detailed status information about a scheduler-driven in-progress
> update. If no update ID is provided, information will be displayed about the
> active update for the job.
> 
> positional arguments:
>   CLUSTER/ROLE/ENV/NAME
>                         Fully specified job key, in CLUSTER/ROLE/ENV/NAME
>                         format
>   ID                    Update identifier provided by the scheduler when an
>                         update was started.
> 
> optional arguments:
>   -h, --help            show this help message and exit
>   --write-json          Generate command output in JSON format
>   --verbose, -v         Show verbose output
>   --skip-hooks hook,hook,...
>                         A comma-separated list of command hook names that
>                         should be skipped. If the hooks cannot be skipped,
>                         then the command will be aborted
> ```
> 
> Prior to this change, the description was only displayed in the parent command's help text.
> 
> 
> Diffs
> -----
> 
>   src/main/python/apache/aurora/client/cli/__init__.py 6a0c129bc5d5dac8d8d393705a69586c9918983d 
>   src/main/python/apache/aurora/client/cli/update.py 830ef4424fe46bc8c14456492f29dea681cf5200 
>   src/test/python/apache/aurora/client/cli/test_supdate.py f9acbdfd65adb252f3059717a6bc1a1f4ba39c44 
>   src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 320c1fbeee0161528745edd38360cd1fd5d53104 
> 
> Diff: https://reviews.apache.org/r/32313/diff/
> 
> 
> Testing
> -------
> 
> I have converted all test cases in `test_supdate.py` to use the 'new style' non-integration testing, which removed a ton of boilerplate.
> 
> I also corrected some holes in the end-to-end tests, wherein `test` and conditions could silently fail.
> 
> 
> Thanks,
> 
> Bill Farner
> 
>


--===============4039062330752257677==--