> On June 27, 2016, 6:31 p.m., Maxim Khutornenko wrote: > > src/main/python/apache/aurora/client/cli/context.py, lines 129-130 > > <https://reviews.apache.org/r/49048/diff/3/?file=1431153#file1431153line129> > > > > Can you simplify this as: > > ``` > > return not to_bool(tier.settings['preemptible']) and not > > to_bool(tier.settings['revocable']) > > ``` > > > > and falback to `tier_configurations.defaultTierName` in the next()? > > > > Also, I'd change the function name to something less generic, like > > `production_tier_filter`. > > Mehrdad Nurolahzade wrote: > Do you mean something like the following? (this is breaking my tests) > ``` > def production_tier_filter(tier): > return not to_bool(tier.settings['preemptible']) and not > to_bool(tier.settings['revocable']) > > task = config.job().taskConfig > if task.tier is None: > backfill_args = { > 'tier': String( > next( > (t.name for t in tier_configurations.tiers if > production_tier_filter(t)), > tier_configurations.defaultTierName)) > } > else: > backfill_args = { > 'production': Boolean( > next( > (not to_bool(t.settings['preemptible']) for t in > tier_configurations.tiers if > production_tier_filter(t)), > task.production)) > } > ```
I presume this is due to the "else" block not accounting for the actual `task.tier` value? > On June 27, 2016, 6:31 p.m., Maxim Khutornenko wrote: > > src/main/python/apache/aurora/client/config.py, line 118 > > <https://reviews.apache.org/r/49048/diff/3/?file=1431154#file1431154line118> > > > > Please, add a link to our docs for more info: > > http://aurora.apache.org/documentation/latest/reference/configuration/#job-objects > > > > Also, release notes need to be updated to clearly state this > > deprecation route. > > Mehrdad Nurolahzade wrote: > We added an entry to ```RELEASE-NOTES.md``` for 0.14.0 under > "Deprecations and removals" with a previous JIRA, do we need anything beyond > that? > ``` > - Deprecated `production` field in `TaskConfig` thrift struct. Use `tier` > field to specify task > scheduling and resource handling behavior. > ``` I would add a separate note explicitly stating the new client behavior: warning message and tier defaulting. This is finally a user-visible change that I think warrants a special mentioning. - Maxim ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49048/#review139597 ----------------------------------------------------------- On June 27, 2016, 4:02 p.m., Mehrdad Nurolahzade wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/49048/ > ----------------------------------------------------------- > > (Updated June 27, 2016, 4:02 p.m.) > > > Review request for Aurora, Joshua Cohen and Maxim Khutornenko. > > > Repository: aurora > > > Description > ------- > > AURORA-1710 Make 'tier' required and remove support for 'production' flag in > Job configuration - CLI changes > > > Diffs > ----- > > src/main/python/apache/aurora/client/api/__init__.py > 68baf8fdb90cd26100159401c46c9963c24332b3 > src/main/python/apache/aurora/client/cli/context.py > 9b1511801d031ff48b81c25688a55cb586b8ac66 > src/main/python/apache/aurora/client/config.py > 2fc12559016d406c347adb416a5166cca31c961e > src/test/python/apache/aurora/client/cli/test_command_hooks.py > 2130f1fa71be02a004cdf8e476a270c81a7105d3 > src/test/python/apache/aurora/client/cli/test_context.py > 204ca092adad8bf43c5032a02f61bf303fb0b2fc > src/test/python/apache/aurora/client/cli/test_create.py > 8c27e2b340bb0a5fb5bcb44ef94d433e7f92c76c > src/test/python/apache/aurora/client/cli/test_cron.py > f3c522ed94a2d774865811ceb546bf9df083c14f > src/test/python/apache/aurora/client/cli/test_plugins.py > a545fece5e2b3e0017a61e1be9ac478372b1f34d > src/test/python/apache/aurora/client/cli/test_restart.py > 967d560e5c7eb0ed85b215fb11d9751b8666acb5 > src/test/python/apache/aurora/client/cli/util.py > 7b4558ec7f0fb0fd2902591bc6a90dc15051dd6e > src/test/python/apache/aurora/client/test_config.py > b1a3c1865819899ef19173be0f861783a2631d0a > > Diff: https://reviews.apache.org/r/49048/diff/ > > > Testing > ------- > > ``` > ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh > > *** OK (All tests passed) *** > > mesos-master start/running, process 26868 > + RETCODE=0 > + restore_netrc > + mv /home/vagrant/.netrc.bak /home/vagrant/.netrc > + true > Connection to 127.0.0.1 closed. > > real 19m46.324s > user 0m1.496s > sys 0m0.774s > ``` > > > Thanks, > > Mehrdad Nurolahzade > >
