> On Dec. 14, 2013, 1:26 a.m., Bill Farner wrote: > > src/main/python/twitter/aurora/client/api/updater.py, line 376 > > <https://reviews.apache.org/r/16268/diff/1/?file=397921#file397921line376> > > > > I think this is unsafe. 'Error' is definitely too generic to determine > > that it's safe to release the lock. Even recognizing that a > > locking-related error was encountered may not be enough, since in the > > future other locks could be held.
The whole point of applying locks is to prevent simultaneous updates. In _get_update_instructions we are just gathering information and not applying any changes. The update has technically not started yet and no mutations were attempted. What's the point of holding the lock at this stage? - Maxim ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16268/#review30393 ----------------------------------------------------------- On Dec. 14, 2013, 1:21 a.m., Maxim Khutornenko wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/16268/ > ----------------------------------------------------------- > > (Updated Dec. 14, 2013, 1:21 a.m.) > > > Review request for Aurora, Bill Farner and Brian Wickman. > > > Repository: aurora > > > Description > ------- > > Releasing the lock if the update is unable to start for any reason. > > > Diffs > ----- > > src/main/python/twitter/aurora/client/api/updater.py > 32bd6a1bf6bf401b0341b6e14e570d83c3e79dd5 > src/test/python/twitter/aurora/client/api/test_updater.py > 0b60d9e04428eb55168e3a9c3a394f19aed2f9d2 > > Diff: https://reviews.apache.org/r/16268/diff/ > > > Testing > ------- > > $ ./pants src/test/python/twitter/aurora/client/api:updater > Build operating on targets: > OrderedSet([PythonTests(src/test/python/twitter/aurora/client/api/BUILD:updater)]) > ============================================================================== > test session starts > =============================================================================== > platform darwin -- Python 2.7.3 -- pytest-2.5.0 > collected 24 items > > src/test/python/twitter/aurora/client/api/test_updater.py > ........................ > > =========================================================================== > 24 passed in 0.26 seconds > ============================================================================ > src.test.python.twitter.aurora.client.api.updater > ..... SUCCESS > > > Thanks, > > Maxim Khutornenko > >
