You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@aurora.apache.org by Brian Wickman <wi...@apache.org> on 2015/06/01 20:05:11 UTC

Re: Review Request 34300: Do better sanitation on the client side when encountering unbound pystachio refs

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

(Updated June 1, 2015, 6:05 p.m.)


Review request for Aurora and Kevin Sweeney.


Changes
-------

Fix checkstyle violation.


Bugs: AURORA-739
    https://issues.apache.org/jira/browse/AURORA-739


Repository: aurora


Description
-------

It's possible to define nested refs that can cause the executor to stack trace, e.g.
{{derp[{{thermos.ports[http]}}]}} is perfectly valid but crashes the executor.


Diffs (updated)
-----

  src/main/python/apache/aurora/config/__init__.py dd2f89014a3da730364b14e01c499ac0f2c288c1 
  src/main/python/apache/aurora/config/schema/base.py a87524a8b3ad5aa0e337e0a0028cecb85865b4e6 
  src/main/python/apache/aurora/config/thrift.py 810febb637d168b07c4aea77984e1d1451a39af2 
  src/main/python/apache/aurora/executor/common/task_info.py d110faf08135d94d9af95ad74613950c56248c09 
  src/main/python/apache/thermos/config/dsl.py 0663a9ad8ae194b63ce6d77dabfe65564e2d26dd 
  src/main/python/apache/thermos/config/loader.py d77ab9a52b16e9d65acdb95f01fd251ae8ab2b6e 
  src/test/python/apache/aurora/client/test_config.py c56779712b91f621261358aa7ebd6c4fc65446a0 
  src/test/python/apache/aurora/config/test_thrift.py 654c0b5ae82c98db163c7a44301ff6b23e19b211 
  src/test/python/apache/aurora/executor/common/test_task_info.py 102ba531aa6c28f2d74bd0d7f1668e5861e3a6b8 

Diff: https://reviews.apache.org/r/34300/diff/


Testing
-------

Added some regression tests.


Thanks,

Brian Wickman


Re: Review Request 34300: Do better sanitation on the client side when encountering unbound pystachio refs

Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34300/#review86028
-----------------------------------------------------------

Ship it!


Master (827b9ab) is green with this patch.
  ./build-support/jenkins/build.sh

I will refresh this build result if you post a review containing "@ReviewBot retry"

- Aurora ReviewBot


On June 1, 2015, 6:05 p.m., Brian Wickman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34300/
> -----------------------------------------------------------
> 
> (Updated June 1, 2015, 6:05 p.m.)
> 
> 
> Review request for Aurora and Kevin Sweeney.
> 
> 
> Bugs: AURORA-739
>     https://issues.apache.org/jira/browse/AURORA-739
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> It's possible to define nested refs that can cause the executor to stack trace, e.g.
> {{derp[{{thermos.ports[http]}}]}} is perfectly valid but crashes the executor.
> 
> 
> Diffs
> -----
> 
>   src/main/python/apache/aurora/config/__init__.py dd2f89014a3da730364b14e01c499ac0f2c288c1 
>   src/main/python/apache/aurora/config/schema/base.py a87524a8b3ad5aa0e337e0a0028cecb85865b4e6 
>   src/main/python/apache/aurora/config/thrift.py 810febb637d168b07c4aea77984e1d1451a39af2 
>   src/main/python/apache/aurora/executor/common/task_info.py d110faf08135d94d9af95ad74613950c56248c09 
>   src/main/python/apache/thermos/config/dsl.py 0663a9ad8ae194b63ce6d77dabfe65564e2d26dd 
>   src/main/python/apache/thermos/config/loader.py d77ab9a52b16e9d65acdb95f01fd251ae8ab2b6e 
>   src/test/python/apache/aurora/client/test_config.py c56779712b91f621261358aa7ebd6c4fc65446a0 
>   src/test/python/apache/aurora/config/test_thrift.py 654c0b5ae82c98db163c7a44301ff6b23e19b211 
>   src/test/python/apache/aurora/executor/common/test_task_info.py 102ba531aa6c28f2d74bd0d7f1668e5861e3a6b8 
> 
> Diff: https://reviews.apache.org/r/34300/diff/
> 
> 
> Testing
> -------
> 
> Added some regression tests.
> 
> 
> Thanks,
> 
> Brian Wickman
> 
>


Re: Review Request 34300: Do better sanitation on the client side when encountering unbound pystachio refs

Posted by Brian Wickman <wi...@apache.org>.

> On June 4, 2015, 11:55 p.m., Kevin Sweeney wrote:
> > src/main/python/apache/aurora/config/thrift.py, line 218
> > <https://reviews.apache.org/r/34300/diff/3/?file=975774#file975774line218>
> >
> >     Why set this at all? Does the scheduler read this field?

it is not.  i will leave it up to AURORA-739 to remove support entirely (or add it back in a sensible way.)


- Brian


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


On June 1, 2015, 6:05 p.m., Brian Wickman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34300/
> -----------------------------------------------------------
> 
> (Updated June 1, 2015, 6:05 p.m.)
> 
> 
> Review request for Aurora and Kevin Sweeney.
> 
> 
> Bugs: AURORA-739
>     https://issues.apache.org/jira/browse/AURORA-739
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> It's possible to define nested refs that can cause the executor to stack trace, e.g.
> {{derp[{{thermos.ports[http]}}]}} is perfectly valid but crashes the executor.
> 
> 
> Diffs
> -----
> 
>   src/main/python/apache/aurora/config/__init__.py dd2f89014a3da730364b14e01c499ac0f2c288c1 
>   src/main/python/apache/aurora/config/schema/base.py a87524a8b3ad5aa0e337e0a0028cecb85865b4e6 
>   src/main/python/apache/aurora/config/thrift.py 810febb637d168b07c4aea77984e1d1451a39af2 
>   src/main/python/apache/aurora/executor/common/task_info.py d110faf08135d94d9af95ad74613950c56248c09 
>   src/main/python/apache/thermos/config/dsl.py 0663a9ad8ae194b63ce6d77dabfe65564e2d26dd 
>   src/main/python/apache/thermos/config/loader.py d77ab9a52b16e9d65acdb95f01fd251ae8ab2b6e 
>   src/test/python/apache/aurora/client/test_config.py c56779712b91f621261358aa7ebd6c4fc65446a0 
>   src/test/python/apache/aurora/config/test_thrift.py 654c0b5ae82c98db163c7a44301ff6b23e19b211 
>   src/test/python/apache/aurora/executor/common/test_task_info.py 102ba531aa6c28f2d74bd0d7f1668e5861e3a6b8 
> 
> Diff: https://reviews.apache.org/r/34300/diff/
> 
> 
> Testing
> -------
> 
> Added some regression tests.
> 
> 
> Thanks,
> 
> Brian Wickman
> 
>


Re: Review Request 34300: Do better sanitation on the client side when encountering unbound pystachio refs

Posted by Kevin Sweeney <ke...@apache.org>.

> On June 4, 2015, 4:55 p.m., Kevin Sweeney wrote:
> > src/main/python/apache/aurora/config/thrift.py, line 218
> > <https://reviews.apache.org/r/34300/diff/3/?file=975774#file975774line218>
> >
> >     Why set this at all? Does the scheduler read this field?
> 
> Brian Wickman wrote:
>     it is not.  i will leave it up to AURORA-739 to remove support entirely (or add it back in a sensible way.)

In that case, please drop this set from the client-side.


- Kevin


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


On June 1, 2015, 11:05 a.m., Brian Wickman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34300/
> -----------------------------------------------------------
> 
> (Updated June 1, 2015, 11:05 a.m.)
> 
> 
> Review request for Aurora and Kevin Sweeney.
> 
> 
> Bugs: AURORA-739
>     https://issues.apache.org/jira/browse/AURORA-739
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> It's possible to define nested refs that can cause the executor to stack trace, e.g.
> {{derp[{{thermos.ports[http]}}]}} is perfectly valid but crashes the executor.
> 
> 
> Diffs
> -----
> 
>   src/main/python/apache/aurora/config/__init__.py dd2f89014a3da730364b14e01c499ac0f2c288c1 
>   src/main/python/apache/aurora/config/schema/base.py a87524a8b3ad5aa0e337e0a0028cecb85865b4e6 
>   src/main/python/apache/aurora/config/thrift.py 810febb637d168b07c4aea77984e1d1451a39af2 
>   src/main/python/apache/aurora/executor/common/task_info.py d110faf08135d94d9af95ad74613950c56248c09 
>   src/main/python/apache/thermos/config/dsl.py 0663a9ad8ae194b63ce6d77dabfe65564e2d26dd 
>   src/main/python/apache/thermos/config/loader.py d77ab9a52b16e9d65acdb95f01fd251ae8ab2b6e 
>   src/test/python/apache/aurora/client/test_config.py c56779712b91f621261358aa7ebd6c4fc65446a0 
>   src/test/python/apache/aurora/config/test_thrift.py 654c0b5ae82c98db163c7a44301ff6b23e19b211 
>   src/test/python/apache/aurora/executor/common/test_task_info.py 102ba531aa6c28f2d74bd0d7f1668e5861e3a6b8 
> 
> Diff: https://reviews.apache.org/r/34300/diff/
> 
> 
> Testing
> -------
> 
> Added some regression tests.
> 
> 
> Thanks,
> 
> Brian Wickman
> 
>


Re: Review Request 34300: Do better sanitation on the client side when encountering unbound pystachio refs

Posted by Kevin Sweeney <ke...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34300/#review86742
-----------------------------------------------------------

Ship it!



src/main/python/apache/aurora/config/thrift.py
<https://reviews.apache.org/r/34300/#comment138805>

    Why set this at all? Does the scheduler read this field?


- Kevin Sweeney


On June 1, 2015, 11:05 a.m., Brian Wickman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34300/
> -----------------------------------------------------------
> 
> (Updated June 1, 2015, 11:05 a.m.)
> 
> 
> Review request for Aurora and Kevin Sweeney.
> 
> 
> Bugs: AURORA-739
>     https://issues.apache.org/jira/browse/AURORA-739
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> It's possible to define nested refs that can cause the executor to stack trace, e.g.
> {{derp[{{thermos.ports[http]}}]}} is perfectly valid but crashes the executor.
> 
> 
> Diffs
> -----
> 
>   src/main/python/apache/aurora/config/__init__.py dd2f89014a3da730364b14e01c499ac0f2c288c1 
>   src/main/python/apache/aurora/config/schema/base.py a87524a8b3ad5aa0e337e0a0028cecb85865b4e6 
>   src/main/python/apache/aurora/config/thrift.py 810febb637d168b07c4aea77984e1d1451a39af2 
>   src/main/python/apache/aurora/executor/common/task_info.py d110faf08135d94d9af95ad74613950c56248c09 
>   src/main/python/apache/thermos/config/dsl.py 0663a9ad8ae194b63ce6d77dabfe65564e2d26dd 
>   src/main/python/apache/thermos/config/loader.py d77ab9a52b16e9d65acdb95f01fd251ae8ab2b6e 
>   src/test/python/apache/aurora/client/test_config.py c56779712b91f621261358aa7ebd6c4fc65446a0 
>   src/test/python/apache/aurora/config/test_thrift.py 654c0b5ae82c98db163c7a44301ff6b23e19b211 
>   src/test/python/apache/aurora/executor/common/test_task_info.py 102ba531aa6c28f2d74bd0d7f1668e5861e3a6b8 
> 
> Diff: https://reviews.apache.org/r/34300/diff/
> 
> 
> Testing
> -------
> 
> Added some regression tests.
> 
> 
> Thanks,
> 
> Brian Wickman
> 
>


Re: Review Request 34300: Do better sanitation on the client side when encountering unbound pystachio refs

Posted by Joe Smith <ya...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34300/#review88188
-----------------------------------------------------------

Ship it!


Ship It!

- Joe Smith


On June 1, 2015, 11:05 a.m., Brian Wickman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34300/
> -----------------------------------------------------------
> 
> (Updated June 1, 2015, 11:05 a.m.)
> 
> 
> Review request for Aurora and Kevin Sweeney.
> 
> 
> Bugs: AURORA-739
>     https://issues.apache.org/jira/browse/AURORA-739
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> It's possible to define nested refs that can cause the executor to stack trace, e.g.
> {{derp[{{thermos.ports[http]}}]}} is perfectly valid but crashes the executor.
> 
> 
> Diffs
> -----
> 
>   src/main/python/apache/aurora/config/__init__.py dd2f89014a3da730364b14e01c499ac0f2c288c1 
>   src/main/python/apache/aurora/config/schema/base.py a87524a8b3ad5aa0e337e0a0028cecb85865b4e6 
>   src/main/python/apache/aurora/config/thrift.py 810febb637d168b07c4aea77984e1d1451a39af2 
>   src/main/python/apache/aurora/executor/common/task_info.py d110faf08135d94d9af95ad74613950c56248c09 
>   src/main/python/apache/thermos/config/dsl.py 0663a9ad8ae194b63ce6d77dabfe65564e2d26dd 
>   src/main/python/apache/thermos/config/loader.py d77ab9a52b16e9d65acdb95f01fd251ae8ab2b6e 
>   src/test/python/apache/aurora/client/test_config.py c56779712b91f621261358aa7ebd6c4fc65446a0 
>   src/test/python/apache/aurora/config/test_thrift.py 654c0b5ae82c98db163c7a44301ff6b23e19b211 
>   src/test/python/apache/aurora/executor/common/test_task_info.py 102ba531aa6c28f2d74bd0d7f1668e5861e3a6b8 
> 
> Diff: https://reviews.apache.org/r/34300/diff/
> 
> 
> Testing
> -------
> 
> Added some regression tests.
> 
> 
> Thanks,
> 
> Brian Wickman
> 
>