Re: Review Request 31350: Fix clusters.patch contextmanager cleanup
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31350/#review75203 --- Stephan: I'm ready to commit this but it looks like you don't have an email address in reviewboard. Can you set one so that we can attribute this patch and ping this review? ``` File /Users/ksweeney/workspace/aurora/build-support/rbt.venv/lib/python2.7/site-packages/rbtools/clients/git.py, line 740, in create_commit '--author=%s %s' % (author.fullname, author.email)]) File /Users/ksweeney/workspace/aurora/build-support/rbt.venv/lib/python2.7/site-packages/rbtools/api/resource.py, line 299, in __getattr__ raise AttributeError AttributeError ``` - Kevin Sweeney On Feb. 24, 2015, 2:12 p.m., Stephan Erb wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31350/ --- (Updated Feb. 24, 2015, 2:12 p.m.) Review request for Aurora, Kevin Sweeney and Maxim Khutornenko. Repository: aurora Description --- Fix clusters.patch contextmanager cleanup Otherwise one failing testcase can affect the entire suite. Diffs - src/main/python/apache/aurora/common/clusters.py c4b7fefca30313b281808478bf23158a9b7fbf85 src/test/python/apache/aurora/common/test_clusters.py 1bd696e9cd28d87d0cac68b33ab043407d796b61 Diff: https://reviews.apache.org/r/31350/diff/ Testing --- ./pants test.pytest --no-fast src/test/python/apache/aurora/common:: Thanks, Stephan Erb
Re: Review Request 31350: Fix clusters.patch contextmanager cleanup
On March 4, 2015, 7:30 p.m., Kevin Sweeney wrote: Stephan: I'm ready to commit this but it looks like you don't have an email address in reviewboard. Can you set one so that we can attribute this patch and ping this review? ``` File /Users/ksweeney/workspace/aurora/build-support/rbt.venv/lib/python2.7/site-packages/rbtools/clients/git.py, line 740, in create_commit '--author=%s %s' % (author.fullname, author.email)]) File /Users/ksweeney/workspace/aurora/build-support/rbt.venv/lib/python2.7/site-packages/rbtools/api/resource.py, line 299, in __getattr__ raise AttributeError AttributeError ``` It should now be public! - Stephan --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31350/#review75203 --- On Feb. 24, 2015, 11:12 p.m., Stephan Erb wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31350/ --- (Updated Feb. 24, 2015, 11:12 p.m.) Review request for Aurora, Kevin Sweeney and Maxim Khutornenko. Repository: aurora Description --- Fix clusters.patch contextmanager cleanup Otherwise one failing testcase can affect the entire suite. Diffs - src/main/python/apache/aurora/common/clusters.py c4b7fefca30313b281808478bf23158a9b7fbf85 src/test/python/apache/aurora/common/test_clusters.py 1bd696e9cd28d87d0cac68b33ab043407d796b61 Diff: https://reviews.apache.org/r/31350/diff/ Testing --- ./pants test.pytest --no-fast src/test/python/apache/aurora/common:: Thanks, Stephan Erb
Re: Review Request 31350: Fix clusters.patch contextmanager cleanup
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31350/ --- (Updated Feb. 24, 2015, 10:52 p.m.) Review request for Aurora. Changes --- Review changes * use pytest.raises to ignore the exception * start with a non-empty cluster * assert clusters content instead of size (as far as possible, given what the class offers) Repository: aurora Description --- Fix clusters.patch contextmanager cleanup Otherwise one failing testcase can affect the entire suite. Diffs (updated) - src/main/python/apache/aurora/common/clusters.py c4b7fefca30313b281808478bf23158a9b7fbf85 src/test/python/apache/aurora/common/test_clusters.py 1bd696e9cd28d87d0cac68b33ab043407d796b61 Diff: https://reviews.apache.org/r/31350/diff/ Testing --- ./pants test.pytest --no-fast src/test/python/apache/aurora/common:: Thanks, Stephan Erb
Re: Review Request 31350: Fix clusters.patch contextmanager cleanup
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31350/#review73792 --- Master (19378c1) is red with this patch. ./build-support/jenkins/build.sh SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/client/cli/test_quota.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/client/cli/test_command_hooks.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/client/cli/test_supdate.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/client/cli/test_cancel_update.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/client/cli/test_config_noun.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/client/cli/test_create.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/client/cli/test_kill.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/client/cli/test_status.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/client/cli/test_diff.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/client/cli/test_inspect.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/client/cli/test_update.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/client/cli/test_plugins.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/client/cli/test_restart.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/client/cli/test_cron.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/client/cli/test_client.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/client/cli/__init__.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/client/cli/test_api_from_cli.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/client/cli/util.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/client/cli/test_version.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/client/cli/test_open.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/client/cli/test_context.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/client/hooks/test_hooked_api.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/client/hooks/test_non_hooked_api.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/client/api/test_sla.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/client/api/test_api.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/client/api/test_health_check.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/client/api/test_job_monitor.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/client/api/test_updater_util.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/client/api/test_scheduler_mux.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/client/api/test_scheduler_client.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/client/api/test_quota_check.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/client/api/test_instance_watcher.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/client/api/test_updater.py Everything Looks Good! SUCCESS:
Re: Review Request 31350: Fix clusters.patch contextmanager cleanup
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31350/#review73815 --- Ship it! Master (19378c1) 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 Feb. 24, 2015, 2:18 p.m., Stephan Erb wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31350/ --- (Updated Feb. 24, 2015, 2:18 p.m.) Review request for Aurora. Repository: aurora Description --- Fix clusters.patch contextmanager cleanup Otherwise one failing testcase can affect the entire suite. Diffs - src/main/python/apache/aurora/common/clusters.py c4b7fefca30313b281808478bf23158a9b7fbf85 src/test/python/apache/aurora/common/test_clusters.py 1bd696e9cd28d87d0cac68b33ab043407d796b61 Diff: https://reviews.apache.org/r/31350/diff/ Testing --- ./pants test.pytest --no-fast src/test/python/apache/aurora/common:: Thanks, Stephan Erb
Re: Review Request 31350: Fix clusters.patch contextmanager cleanup
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31350/#review73806 --- @ReviewBot retry - Stephan Erb On Feb. 24, 2015, 11:57 a.m., Stephan Erb wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31350/ --- (Updated Feb. 24, 2015, 11:57 a.m.) Review request for Aurora. Repository: aurora Description --- Fix clusters.patch contextmanager cleanup Otherwise one failing testcase can affect the entire suite. Diffs - src/main/python/apache/aurora/common/clusters.py c4b7fefca30313b281808478bf23158a9b7fbf85 src/test/python/apache/aurora/common/test_clusters.py 1bd696e9cd28d87d0cac68b33ab043407d796b61 Diff: https://reviews.apache.org/r/31350/diff/ Testing --- ./pants test.pytest --no-fast src/test/python/apache/aurora/common:: Thanks, Stephan Erb
Re: Review Request 31350: Fix clusters.patch contextmanager cleanup
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31350/ --- (Updated Feb. 24, 2015, 3:18 p.m.) Review request for Aurora. Changes --- Fix import order Repository: aurora Description --- Fix clusters.patch contextmanager cleanup Otherwise one failing testcase can affect the entire suite. Diffs (updated) - src/main/python/apache/aurora/common/clusters.py c4b7fefca30313b281808478bf23158a9b7fbf85 src/test/python/apache/aurora/common/test_clusters.py 1bd696e9cd28d87d0cac68b33ab043407d796b61 Diff: https://reviews.apache.org/r/31350/diff/ Testing --- ./pants test.pytest --no-fast src/test/python/apache/aurora/common:: Thanks, Stephan Erb
Re: Review Request 31350: Fix clusters.patch contextmanager cleanup
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31350/#review73832 --- src/test/python/apache/aurora/common/test_clusters.py https://reviews.apache.org/r/31350/#comment120234 How about populating with a real cluster here and asserting it later? src/test/python/apache/aurora/common/test_clusters.py https://reviews.apache.org/r/31350/#comment120235 `with pytest.raises():` should be more compact. src/test/python/apache/aurora/common/test_clusters.py https://reviews.apache.org/r/31350/#comment120237 Please, assert the entire contents here, e.g. `assert clusters == [Cluster(...)]` - Maxim Khutornenko On Feb. 24, 2015, 2:18 p.m., Stephan Erb wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31350/ --- (Updated Feb. 24, 2015, 2:18 p.m.) Review request for Aurora. Repository: aurora Description --- Fix clusters.patch contextmanager cleanup Otherwise one failing testcase can affect the entire suite. Diffs - src/main/python/apache/aurora/common/clusters.py c4b7fefca30313b281808478bf23158a9b7fbf85 src/test/python/apache/aurora/common/test_clusters.py 1bd696e9cd28d87d0cac68b33ab043407d796b61 Diff: https://reviews.apache.org/r/31350/diff/ Testing --- ./pants test.pytest --no-fast src/test/python/apache/aurora/common:: Thanks, Stephan Erb
Re: Review Request 31350: Fix clusters.patch contextmanager cleanup
On Feb. 24, 2015, 11:12 a.m., Kevin Sweeney wrote: This is awesome! If this fixes the full suite, can you also remove `--no-fast` from `build-support/jenkins/build.sh`? Stephan Erb wrote: I doubt that this patch is sufficient to warrant the change of the build script. There may be many more error conditions requiring a `no-fast`. Background info: We are directly using the python client in our own backend and are also writing some integration tests against it. In some of these testcases we are using CLUSTERS.patch and recently discovered its faulty cleanup behaviour. That's unfortunate. Filed https://issues.apache.org/jira/browse/AURORA-1145 to follow-up. - Kevin --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31350/#review73867 --- On Feb. 24, 2015, 2:12 p.m., Stephan Erb wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31350/ --- (Updated Feb. 24, 2015, 2:12 p.m.) Review request for Aurora, Kevin Sweeney and Maxim Khutornenko. Repository: aurora Description --- Fix clusters.patch contextmanager cleanup Otherwise one failing testcase can affect the entire suite. Diffs - src/main/python/apache/aurora/common/clusters.py c4b7fefca30313b281808478bf23158a9b7fbf85 src/test/python/apache/aurora/common/test_clusters.py 1bd696e9cd28d87d0cac68b33ab043407d796b61 Diff: https://reviews.apache.org/r/31350/diff/ Testing --- ./pants test.pytest --no-fast src/test/python/apache/aurora/common:: Thanks, Stephan Erb
Re: Review Request 31350: Fix clusters.patch contextmanager cleanup
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31350/#review73992 --- Ship it! I can only wonder how many times this has caused an error for me... thank you!! :) - Joe Smith On Feb. 24, 2015, 2:12 p.m., Stephan Erb wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31350/ --- (Updated Feb. 24, 2015, 2:12 p.m.) Review request for Aurora, Kevin Sweeney and Maxim Khutornenko. Repository: aurora Description --- Fix clusters.patch contextmanager cleanup Otherwise one failing testcase can affect the entire suite. Diffs - src/main/python/apache/aurora/common/clusters.py c4b7fefca30313b281808478bf23158a9b7fbf85 src/test/python/apache/aurora/common/test_clusters.py 1bd696e9cd28d87d0cac68b33ab043407d796b61 Diff: https://reviews.apache.org/r/31350/diff/ Testing --- ./pants test.pytest --no-fast src/test/python/apache/aurora/common:: Thanks, Stephan Erb
Re: Review Request 31350: Fix clusters.patch contextmanager cleanup
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31350/#review73928 --- Ship it! Master (cd681d9) 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 Feb. 24, 2015, 10:12 p.m., Stephan Erb wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31350/ --- (Updated Feb. 24, 2015, 10:12 p.m.) Review request for Aurora, Kevin Sweeney and Maxim Khutornenko. Repository: aurora Description --- Fix clusters.patch contextmanager cleanup Otherwise one failing testcase can affect the entire suite. Diffs - src/main/python/apache/aurora/common/clusters.py c4b7fefca30313b281808478bf23158a9b7fbf85 src/test/python/apache/aurora/common/test_clusters.py 1bd696e9cd28d87d0cac68b33ab043407d796b61 Diff: https://reviews.apache.org/r/31350/diff/ Testing --- ./pants test.pytest --no-fast src/test/python/apache/aurora/common:: Thanks, Stephan Erb
Re: Review Request 31350: Fix clusters.patch contextmanager cleanup
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31350/#review73939 --- Ship it! src/test/python/apache/aurora/common/test_clusters.py https://reviews.apache.org/r/31350/#comment120356 I'm not sure this test case is valuable since it will run in a non-deterministic order. - Kevin Sweeney On Feb. 24, 2015, 2:12 p.m., Stephan Erb wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31350/ --- (Updated Feb. 24, 2015, 2:12 p.m.) Review request for Aurora, Kevin Sweeney and Maxim Khutornenko. Repository: aurora Description --- Fix clusters.patch contextmanager cleanup Otherwise one failing testcase can affect the entire suite. Diffs - src/main/python/apache/aurora/common/clusters.py c4b7fefca30313b281808478bf23158a9b7fbf85 src/test/python/apache/aurora/common/test_clusters.py 1bd696e9cd28d87d0cac68b33ab043407d796b61 Diff: https://reviews.apache.org/r/31350/diff/ Testing --- ./pants test.pytest --no-fast src/test/python/apache/aurora/common:: Thanks, Stephan Erb
Re: Review Request 31350: Fix clusters.patch contextmanager cleanup
On Feb. 24, 2015, 8:12 p.m., Kevin Sweeney wrote: This is awesome! If this fixes the full suite, can you also remove `--no-fast` from `build-support/jenkins/build.sh`? I doubt that this patch is sufficient to warrant the change of the build script. There may be many more error conditions requiring a `no-fast`. Background info: We are directly using the python client in our own backend and are also writing some integration tests against it. In some of these testcases we are using CLUSTERS.patch and recently discovered its faulty cleanup behaviour. - Stephan --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31350/#review73867 --- On Feb. 24, 2015, 10:52 p.m., Stephan Erb wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31350/ --- (Updated Feb. 24, 2015, 10:52 p.m.) Review request for Aurora, Kevin Sweeney and Maxim Khutornenko. Repository: aurora Description --- Fix clusters.patch contextmanager cleanup Otherwise one failing testcase can affect the entire suite. Diffs - src/main/python/apache/aurora/common/clusters.py c4b7fefca30313b281808478bf23158a9b7fbf85 src/test/python/apache/aurora/common/test_clusters.py 1bd696e9cd28d87d0cac68b33ab043407d796b61 Diff: https://reviews.apache.org/r/31350/diff/ Testing --- ./pants test.pytest --no-fast src/test/python/apache/aurora/common:: Thanks, Stephan Erb
