> On Jan. 16, 2014, 9:29 p.m., Kevin Sweeney wrote:
> > build.gradle, line 293
> > <https://reviews.apache.org/r/16986/diff/1/?file=424971#file424971line293>
> >
> >     should be bash - executable should match #!

Fixed.


> On Jan. 16, 2014, 9:29 p.m., Kevin Sweeney wrote:
> > src/test/org/apache/aurora/verify_thrift_checksum.sh, line 9
> > <https://reviews.apache.org/r/16986/diff/1/?file=424972#file424972line9>
> >
> >     s/in verify mode/in reset mode/

Thanks, fixed.


> On Jan. 16, 2014, 9:29 p.m., Kevin Sweeney wrote:
> > src/test/org/apache/aurora/verify_thrift_checksum.sh, line 23
> > <https://reviews.apache.org/r/16986/diff/1/?file=424972#file424972line23>
> >
> >     Single space after '.'?

I'll grit my teeth and give in.  We double-spacers may be few but we are proud 
:-)


> On Jan. 16, 2014, 9:29 p.m., Kevin Sweeney wrote:
> > src/test/org/apache/aurora/verify_thrift_checksum.sh, line 30
> > <https://reviews.apache.org/r/16986/diff/1/?file=424972#file424972line30>
> >
> >     Actually thinking a bit more this means we'll have an implicit 
> > build-time openssl dependency. As ugly as it is it's probably better to use 
> > a python one-liner here (tested on Python 2.4 and Python 3.3):
> >     
> >     checksum=$(python -c "import hashlib; m=hashlib.md5(); 
> > m.update(open('$file', 'rb').read()); print(m.hexdigest())")
> 
> Jake Farrell wrote:
>     +1, agree with avoiding the openssl dep in favor of python alternative

sgtm, fixed


- Bill


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


On Jan. 16, 2014, 8:34 p.m., Bill Farner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16986/
> -----------------------------------------------------------
> 
> (Updated Jan. 16, 2014, 8:34 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney, Maxim Khutornenko, and Brian 
> Wickman.
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Add a test to encourage taking care with thrift changes.
> 
> We have more to do w.r.t. automating detection and proper handling of schema 
> changes (specifically in the scheduler).  However, i see this as low-hanging 
> fruit to at least serve as a reminder.
> 
> 
> Diffs
> -----
> 
>   build.gradle b4b055c3ae4dd1764d69033d9a666c49c770e042 
>   src/test/org/apache/aurora/verify_thrift_checksum.sh PRE-CREATION 
>   src/test/resources/org/apache/aurora/gen/api.thrift.md5 PRE-CREATION 
>   src/test/resources/org/apache/aurora/gen/internal_rpc.thrift.md5 
> PRE-CREATION 
>   src/test/resources/org/apache/aurora/gen/storage.thrift.md5 PRE-CREATION 
>   src/test/resources/org/apache/aurora/gen/storage_local.thrift.md5 
> PRE-CREATION 
>   src/test/resources/org/apache/aurora/gen/test.thrift.md5 PRE-CREATION 
>   src/test/resources/org/apache/thermos/thermos_internal.thrift.md5 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/16986/diff/
> 
> 
> Testing
> -------
> 
> Simulating a mis-matched md5:
>     $ echo 'foobar' > 
> src/test/resources/org/apache/thermos/thermos_internal.thrift.md5
>     
>     $ ./gradlew build
>     make: Nothing to be done for `all'.
>     Golden checksum did not match for 
> src/main/thrift/org/apache/thermos/thermos_internal.thrift
>     Found a8e77d5255804a8a745d20ca53b6aeda, expected foobar
> 
>     
> !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
>     This means you changed a thrift file.
>     Please think carefully before you proceed!
> 
>     If you are changing an API or a storage schema you may need to
>     take additional actions to such as providing a client and/or
>     server-side migration strategy.  You may also need to bump the
>     released version ID, following the guidelines at http://semver.org
> 
>     This test is not here to help you make those changes, but to
>     remind you to take appropriate follow-up actions relating to your
>     schema change.
> 
>     Once you are confident that you have appropriately supported this
>     change in relevant code, you can fix this test by running
>     sh src/test/sh/verify_thrift_checksum.sh -r
>     
> !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
> 
>     FAILURE: Build failed with an exception.
> 
>     * Where:
>     Build file '/Users/wfarner/Code/aurora/build.gradle' line: 292
> 
>     * What went wrong:
>     A problem occurred evaluating root project 'aurora'.
>     > Process 'command 'sh'' finished with non-zero exit value 1
> 
>     * Try:
>     Run with --stacktrace option to get the stack trace. Run with --info or 
> --debug option to get more log output.
> 
>     BUILD FAILED
> 
>     Total time: 5.783 secs
> 
> Fixing the break:
> 
>     $ sh src/test/sh/verify_thrift_checksum.sh -r
>     Warning: Operating in reset mode.  Rewriting golden checksum files.
> 
>     $ ./gradlew build
>     make: Nothing to be done for `all'.
>     All checksums match.
>     :about
>     :bootstrapThrift UP-TO-DATE
>     :generateSources UP-TO-DATE
>     :compileGeneratedJava UP-TO-DATE
>     :processGeneratedResources UP-TO-DATE
>     :generatedClasses UP-TO-DATE
>     :compileJava UP-TO-DATE
>     :processResources UP-TO-DATE
>     :classes UP-TO-DATE
>     :jar
>     :assemble
>     :checkstyleMain UP-TO-DATE
>     :compileTestJava UP-TO-DATE
>     :processTestResources UP-TO-DATE
>     :testClasses UP-TO-DATE
>     :checkstyleTest UP-TO-DATE
>     :test UP-TO-DATE
>     :jacocoTestReport UP-TO-DATE
>     :check UP-TO-DATE
>     :build
> 
>     BUILD SUCCESSFUL
> 
>     Total time: 15.495 secs
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Reply via email to