> On Oct. 17, 2017, 9: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). > > Stephan Erb wrote: > 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 Erb wrote: > I agree thought hat a specific `"AuroraExecutor"` mode would be the best > thing. My proposal is more like a very easy workaround.
Even if it's pretty printed, the issue is that because it's a string the entire string is considered a diff and you will just see two huge red/green blobs in the output. What we want is to deserialize the JSON and only see the parts which have actually change. All we need is the confidence to do JSON.parse on the executor data. - David ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63083/#review188390 ----------------------------------------------------------- On Oct. 17, 2017, 8: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, 8: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 > >
