aurora-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Stephan Erb <s...@apache.org>
Subject Re: Review Request 63083: Add diff viewer to Update Page
Date Tue, 17 Oct 2017 21:47:39 GMT


> On Oct. 17, 2017, 11:03 p.m., Stephan Erb wrote:
> > Should we consider pretty printing the executor configuration (when assembling the
thrift struct in the client)? Hopefully this would yield diffs that are a bit easier to read.
> 
> David McLaughlin wrote:
>     Currently that would have to assume the executor config is JSON.
> 
> David McLaughlin wrote:
>     But my long term plan is to look for "AuroraExecutor" in the config and do nice things
for Thermos (and allow other people to drop in their own parsers for custom executors).

That is why I'd propose to do it here: https://github.com/apache/aurora/blob/master/src/main/python/apache/aurora/config/thrift.py#L328

At that point in time we are sure this is a Thermos config in json. As it is just a blob for
Aurora, formatting it should not really matter.


- Stephan


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


On Oct. 17, 2017, 10:11 p.m., David McLaughlin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63083/
> -----------------------------------------------------------
> 
> (Updated Oct. 17, 2017, 10:11 p.m.)
> 
> 
> Review request for Aurora, Kai Huang, Reza Motamedi, and Santhosh Kumar Shanmugham.
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Add diff viewer from Job Page to Update Page. Key difference on update page is that the
"after" diff is fixed - it's always the desiredState. But I've extracted the Diff rendering
into a shared component between ConfigDiff and UpdateDiff, as well as the CSS.
> 
> 
> Diffs
> -----
> 
>   ui/src/main/js/components/ConfigDiff.js 9627751293ff01c9a3b728e2c352894f1a1e22f3 
>   ui/src/main/js/components/Diff.js PRE-CREATION 
>   ui/src/main/js/components/InstanceViz.js 99efec4c15fcc677820b3145f506997ef8a37207 
>   ui/src/main/js/components/UpdateConfig.js fac3c8856f3fdcb918fe82ac7d61cf0e53b23756

>   ui/src/main/js/components/UpdateDiff.js PRE-CREATION 
>   ui/src/main/js/components/__tests__/ConfigDiff-test.js 3eaa78a9660e1e4bac284d545f5fae05c24e93f7

>   ui/src/main/js/components/__tests__/Diff-test.js PRE-CREATION 
>   ui/src/main/js/components/__tests__/UpdateDiff-test.js PRE-CREATION 
>   ui/src/main/js/test-utils/UpdateBuilders.js 2764f0e1d6446e5b3aa99d1dc1a952a20efa4798

>   ui/src/main/sass/app.scss 3a799b658ad6c94144cda7beec38c4b72ec393d8 
>   ui/src/main/sass/components/_diff.scss PRE-CREATION 
>   ui/src/main/sass/components/_job-page.scss 8523fcf4c917b46c9d514325f073bd5788798156

>   ui/src/main/sass/components/_update-page.scss a0db0b43b61e8ef10c29195945e7543a982aeab1

> 
> 
> Diff: https://reviews.apache.org/r/63083/diff/1/
> 
> 
> Testing
> -------
> 
> ./gradlew ui:lint
> ./gradlew ui:test
> 
> See screenshots.
> 
> 
> File Attachments
> ----------------
> 
> Selecting from multiple initial configs
>   https://reviews.apache.org/media/uploaded/files/2017/10/17/df437cfa-a97c-4aa1-9a95-2e4291020b9b__Screen_Shot_2017-10-17_at_11.22.18_AM.png
> Showing diff by default
>   https://reviews.apache.org/media/uploaded/files/2017/10/17/740cc0a0-9ac7-413a-8a33-fe452f998400__Screen_Shot_2017-10-17_at_1.09.32_PM.png
> 
> 
> Thanks,
> 
> David McLaughlin
> 
>


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