Re: Review Request 26239: Add usernames to scheduler update operations.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26239/#review55110 --- src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java https://reviews.apache.org/r/26239/#comment95471 please doc new @params on javdocs throughout src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java https://reviews.apache.org/r/26239/#comment95472 Since you allow an absent value, use OptionalString Ditt down the call stack. src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateEventMapper.xml https://reviews.apache.org/r/26239/#comment95473 How does this present when the value is null? Empty string? src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java https://reviews.apache.org/r/26239/#comment95474 line break above, to visually separate the wrapped signature from method body. - Bill Farner On Oct. 1, 2014, 6:42 p.m., David McLaughlin wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26239/ --- (Updated Oct. 1, 2014, 6:42 p.m.) Review request for Aurora, Maxim Khutornenko and Bill Farner. Bugs: AURORA-772 https://issues.apache.org/jira/browse/AURORA-772 Repository: aurora Description --- Add usernames to scheduler update operations. Diffs - src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 01fc345344e4ae807607f8f87e8a9974c3b69151 src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java b8dafe077999c1f2d14bbc260c83386020460396 src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java faa21363b87505e4212574bb9872d1e03a0e8f24 src/main/resources/org/apache/aurora/scheduler/http/ui/update.html aaff5b21f3d20f00eaf98c29d0e5bf19b25f2f62 src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml 631ab2543dadb77b7fd1cae96adc9c0bd2f94b39 src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateEventMapper.xml 9b7e8ba620b42cfb404c9c1440f953918c73 src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 2894b617af082bfde1d44571868200271b38724d src/main/thrift/org/apache/aurora/gen/api.thrift a1217edbcd36cbe02b09a549b71e87ee40ffc6c7 src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java 1b1160f349d64bbcd4d20103a82f3b8eb1ca75d9 src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 60c1582d4211b79656797a84ca6a7a67c7fecdfe src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 8843990484756664a0c16c61303f1aa992e7686d Diff: https://reviews.apache.org/r/26239/diff/ Testing --- ./gradlew -Pq build Thanks, David McLaughlin
Re: Review Request 26239: Add usernames to scheduler update operations.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26239/ --- (Updated Oct. 1, 2014, 7:35 p.m.) Review request for Aurora, Maxim Khutornenko and Bill Farner. Bugs: AURORA-772 https://issues.apache.org/jira/browse/AURORA-772 Repository: aurora Description --- Add usernames to scheduler update operations. Diffs (updated) - src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 01fc345344e4ae807607f8f87e8a9974c3b69151 src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java b8dafe077999c1f2d14bbc260c83386020460396 src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java faa21363b87505e4212574bb9872d1e03a0e8f24 src/main/resources/org/apache/aurora/scheduler/http/ui/update.html aaff5b21f3d20f00eaf98c29d0e5bf19b25f2f62 src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml 631ab2543dadb77b7fd1cae96adc9c0bd2f94b39 src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateEventMapper.xml 9b7e8ba620b42cfb404c9c1440f953918c73 src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 2894b617af082bfde1d44571868200271b38724d src/main/thrift/org/apache/aurora/gen/api.thrift a1217edbcd36cbe02b09a549b71e87ee40ffc6c7 src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java 1b1160f349d64bbcd4d20103a82f3b8eb1ca75d9 src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 60c1582d4211b79656797a84ca6a7a67c7fecdfe src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 8843990484756664a0c16c61303f1aa992e7686d Diff: https://reviews.apache.org/r/26239/diff/ Testing --- ./gradlew -Pq build Thanks, David McLaughlin
Re: Review Request 26239: Add usernames to scheduler update operations.
On Oct. 1, 2014, 6:49 p.m., Bill Farner wrote: src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java, line 688 https://reviews.apache.org/r/26239/diff/1/?file=710198#file710198line688 line break above, to visually separate the wrapped signature from method body. David McLaughlin wrote: And of course I based my style on the nearest existing method, which didn't have that. Fixed both of them. Sorry about that, this is one thing that i have not been successful at encoding in a checkstyle rule :-/ On Oct. 1, 2014, 6:49 p.m., Bill Farner wrote: src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateEventMapper.xml, line 33 https://reviews.apache.org/r/26239/diff/1/?file=710195#file710195line33 How does this present when the value is null? Empty string? David McLaughlin wrote: Yes. The frustrating thing is we can't assume null = system events, because for backwards compatibility we need to account for existing update events from before this patch having null users too. That's fine, just wanted to make sure it's not 'null'. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26239/#review55110 --- On Oct. 1, 2014, 7:35 p.m., David McLaughlin wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26239/ --- (Updated Oct. 1, 2014, 7:35 p.m.) Review request for Aurora, Maxim Khutornenko and Bill Farner. Bugs: AURORA-772 https://issues.apache.org/jira/browse/AURORA-772 Repository: aurora Description --- Add usernames to scheduler update operations. Diffs - src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 01fc345344e4ae807607f8f87e8a9974c3b69151 src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java b8dafe077999c1f2d14bbc260c83386020460396 src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java faa21363b87505e4212574bb9872d1e03a0e8f24 src/main/resources/org/apache/aurora/scheduler/http/ui/update.html aaff5b21f3d20f00eaf98c29d0e5bf19b25f2f62 src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml 631ab2543dadb77b7fd1cae96adc9c0bd2f94b39 src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateEventMapper.xml 9b7e8ba620b42cfb404c9c1440f953918c73 src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 2894b617af082bfde1d44571868200271b38724d src/main/thrift/org/apache/aurora/gen/api.thrift a1217edbcd36cbe02b09a549b71e87ee40ffc6c7 src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java 1b1160f349d64bbcd4d20103a82f3b8eb1ca75d9 src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 60c1582d4211b79656797a84ca6a7a67c7fecdfe src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 8843990484756664a0c16c61303f1aa992e7686d Diff: https://reviews.apache.org/r/26239/diff/ Testing --- ./gradlew -Pq build Thanks, David McLaughlin
Re: Review Request 26239: Add usernames to scheduler update operations.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26239/#review55154 --- Ship it! src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java https://reviews.apache.org/r/26239/#comment95536 I'd actually opt for orNull() here. An empty string might be confused with the auth system spitting out empty. - Bill Farner On Oct. 1, 2014, 7:35 p.m., David McLaughlin wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26239/ --- (Updated Oct. 1, 2014, 7:35 p.m.) Review request for Aurora, Maxim Khutornenko and Bill Farner. Bugs: AURORA-772 https://issues.apache.org/jira/browse/AURORA-772 Repository: aurora Description --- Add usernames to scheduler update operations. Diffs - src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 01fc345344e4ae807607f8f87e8a9974c3b69151 src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java b8dafe077999c1f2d14bbc260c83386020460396 src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java faa21363b87505e4212574bb9872d1e03a0e8f24 src/main/resources/org/apache/aurora/scheduler/http/ui/update.html aaff5b21f3d20f00eaf98c29d0e5bf19b25f2f62 src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml 631ab2543dadb77b7fd1cae96adc9c0bd2f94b39 src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateEventMapper.xml 9b7e8ba620b42cfb404c9c1440f953918c73 src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 2894b617af082bfde1d44571868200271b38724d src/main/thrift/org/apache/aurora/gen/api.thrift a1217edbcd36cbe02b09a549b71e87ee40ffc6c7 src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java 1b1160f349d64bbcd4d20103a82f3b8eb1ca75d9 src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 60c1582d4211b79656797a84ca6a7a67c7fecdfe src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 8843990484756664a0c16c61303f1aa992e7686d Diff: https://reviews.apache.org/r/26239/diff/ Testing --- ./gradlew -Pq build Thanks, David McLaughlin
Re: Review Request 26239: Add usernames to scheduler update operations.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26239/#review55175 --- Ship it! src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java https://reviews.apache.org/r/26239/#comment95561 s/id/name/g - Maxim Khutornenko On Oct. 1, 2014, 7:35 p.m., David McLaughlin wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26239/ --- (Updated Oct. 1, 2014, 7:35 p.m.) Review request for Aurora, Maxim Khutornenko and Bill Farner. Bugs: AURORA-772 https://issues.apache.org/jira/browse/AURORA-772 Repository: aurora Description --- Add usernames to scheduler update operations. Diffs - src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 01fc345344e4ae807607f8f87e8a9974c3b69151 src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java b8dafe077999c1f2d14bbc260c83386020460396 src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java faa21363b87505e4212574bb9872d1e03a0e8f24 src/main/resources/org/apache/aurora/scheduler/http/ui/update.html aaff5b21f3d20f00eaf98c29d0e5bf19b25f2f62 src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml 631ab2543dadb77b7fd1cae96adc9c0bd2f94b39 src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateEventMapper.xml 9b7e8ba620b42cfb404c9c1440f953918c73 src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 2894b617af082bfde1d44571868200271b38724d src/main/thrift/org/apache/aurora/gen/api.thrift a1217edbcd36cbe02b09a549b71e87ee40ffc6c7 src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java 1b1160f349d64bbcd4d20103a82f3b8eb1ca75d9 src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 60c1582d4211b79656797a84ca6a7a67c7fecdfe src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 8843990484756664a0c16c61303f1aa992e7686d Diff: https://reviews.apache.org/r/26239/diff/ Testing --- ./gradlew -Pq build Thanks, David McLaughlin
Re: Review Request 26239: Add usernames to scheduler update operations.
On Oct. 2, 2014, 1:29 a.m., Maxim Khutornenko wrote: src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java, line 53 https://reviews.apache.org/r/26239/diff/2/?file=710308#file710308line53 s/id/name/g Fixed. - David --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26239/#review55175 --- On Oct. 2, 2014, 2:29 a.m., David McLaughlin wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26239/ --- (Updated Oct. 2, 2014, 2:29 a.m.) Review request for Aurora, Maxim Khutornenko and Bill Farner. Bugs: AURORA-772 https://issues.apache.org/jira/browse/AURORA-772 Repository: aurora Description --- Add usernames to scheduler update operations. Diffs - src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 01fc345344e4ae807607f8f87e8a9974c3b69151 src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java b8dafe077999c1f2d14bbc260c83386020460396 src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java faa21363b87505e4212574bb9872d1e03a0e8f24 src/main/resources/org/apache/aurora/scheduler/http/ui/update.html aaff5b21f3d20f00eaf98c29d0e5bf19b25f2f62 src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml 631ab2543dadb77b7fd1cae96adc9c0bd2f94b39 src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateEventMapper.xml 9b7e8ba620b42cfb404c9c1440f953918c73 src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 2894b617af082bfde1d44571868200271b38724d src/main/thrift/org/apache/aurora/gen/api.thrift a1217edbcd36cbe02b09a549b71e87ee40ffc6c7 src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java 1b1160f349d64bbcd4d20103a82f3b8eb1ca75d9 src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 60c1582d4211b79656797a84ca6a7a67c7fecdfe src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 8843990484756664a0c16c61303f1aa992e7686d Diff: https://reviews.apache.org/r/26239/diff/ Testing --- ./gradlew -Pq build Thanks, David McLaughlin
Re: Review Request 26239: Add usernames to scheduler update operations.
On Oct. 1, 2014, 10:42 p.m., Bill Farner wrote: src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java, line 383 https://reviews.apache.org/r/26239/diff/2/?file=710309#file710309line383 I'd actually opt for orNull() here. An empty string might be confused with the auth system spitting out empty. Fixed. - David --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26239/#review55154 --- On Oct. 2, 2014, 2:29 a.m., David McLaughlin wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26239/ --- (Updated Oct. 2, 2014, 2:29 a.m.) Review request for Aurora, Maxim Khutornenko and Bill Farner. Bugs: AURORA-772 https://issues.apache.org/jira/browse/AURORA-772 Repository: aurora Description --- Add usernames to scheduler update operations. Diffs - src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 01fc345344e4ae807607f8f87e8a9974c3b69151 src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java b8dafe077999c1f2d14bbc260c83386020460396 src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java faa21363b87505e4212574bb9872d1e03a0e8f24 src/main/resources/org/apache/aurora/scheduler/http/ui/update.html aaff5b21f3d20f00eaf98c29d0e5bf19b25f2f62 src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml 631ab2543dadb77b7fd1cae96adc9c0bd2f94b39 src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateEventMapper.xml 9b7e8ba620b42cfb404c9c1440f953918c73 src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 2894b617af082bfde1d44571868200271b38724d src/main/thrift/org/apache/aurora/gen/api.thrift a1217edbcd36cbe02b09a549b71e87ee40ffc6c7 src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java 1b1160f349d64bbcd4d20103a82f3b8eb1ca75d9 src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 60c1582d4211b79656797a84ca6a7a67c7fecdfe src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 8843990484756664a0c16c61303f1aa992e7686d Diff: https://reviews.apache.org/r/26239/diff/ Testing --- ./gradlew -Pq build Thanks, David McLaughlin
Re: Review Request 26239: Add usernames to scheduler update operations.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26239/ --- (Updated Oct. 2, 2014, 2:29 a.m.) Review request for Aurora, Maxim Khutornenko and Bill Farner. Changes --- rb feedback. Bugs: AURORA-772 https://issues.apache.org/jira/browse/AURORA-772 Repository: aurora Description --- Add usernames to scheduler update operations. Diffs (updated) - src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 01fc345344e4ae807607f8f87e8a9974c3b69151 src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java b8dafe077999c1f2d14bbc260c83386020460396 src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java faa21363b87505e4212574bb9872d1e03a0e8f24 src/main/resources/org/apache/aurora/scheduler/http/ui/update.html aaff5b21f3d20f00eaf98c29d0e5bf19b25f2f62 src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml 631ab2543dadb77b7fd1cae96adc9c0bd2f94b39 src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateEventMapper.xml 9b7e8ba620b42cfb404c9c1440f953918c73 src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 2894b617af082bfde1d44571868200271b38724d src/main/thrift/org/apache/aurora/gen/api.thrift a1217edbcd36cbe02b09a549b71e87ee40ffc6c7 src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java 1b1160f349d64bbcd4d20103a82f3b8eb1ca75d9 src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 60c1582d4211b79656797a84ca6a7a67c7fecdfe src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 8843990484756664a0c16c61303f1aa992e7686d Diff: https://reviews.apache.org/r/26239/diff/ Testing --- ./gradlew -Pq build Thanks, David McLaughlin
