Re: Review Request 22065: Fix /cron endpoint.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22065/#review44341 --- Ship it! Ship It! - Suman Karumuri On May 30, 2014, 1:32 a.m., Kevin Sweeney wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22065/ --- (Updated May 30, 2014, 1:32 a.m.) Review request for Aurora, David McLaughlin and Bill Farner. Bugs: AURORA-478 https://issues.apache.org/jira/browse/AURORA-478 Repository: aurora Description --- Fix /cron endpoint. Diffs - src/main/java/org/apache/aurora/scheduler/cron/quartz/CronJobManagerImpl.java 3359425b5f19e68f33c46c5191ad100c1857d978 src/main/java/org/apache/aurora/scheduler/http/Cron.java 6ccf5833012b4282e6a0fc94db39ac4ccd5ce77f src/test/java/org/apache/aurora/scheduler/cron/quartz/CronJobManagerImplTest.java efa0a583dfa63ae1240a2fff128d97497c83536e src/test/java/org/apache/aurora/scheduler/http/CronTest.java PRE-CREATION Diff: https://reviews.apache.org/r/22065/diff/ Testing --- ./gradlew -Pq build ./gradlew run curl -s http://localhost:8081/cron | python -m json.tool Thanks, Kevin Sweeney
Re: Review Request 22008: Exclude .git from rsync copy in vagrant.
On May 29, 2014, 7:58 a.m., Mark Chu-Carroll wrote: Why? Right now, I do a lot of debugging of the client using vagrant. My typical workflow is: - Make the change in my git workspace. - Commit it to a branch. - vagrant ssh into the virtual cluster - git pull /vagrant mybranch - build client - test If we eliminate that .git file, then the aurora copy in the vagrant host can no longer be used for pulls. We can't just use the /vagrant copy, because that's got a ton of state. With this change, to do a similar dev workflow, I'd need to create a git workspace cloned from /vagrant, and also add new stuff to copy the resulting aurora/aurora2 pexes into the original /vagrant copy. It makes the workflow more complicated, and I'm not clear on what benefits it produces in exchange. Bill Farner wrote: The workflow would be slightly different than you describe. To update builds in the VM, you would run vagrant provision, and rsync will pick up the changes. Does that sound reasonable? Mark Chu-Carroll wrote: It's a *lot* slower. Trying it out with a simple change in the client, the round-trip time for a change using git is under five seconds, for git commit, git pull, and pants to build the client. Running vagrant provision took one minute and 20 seconds for an equivalent process. It's just a minute - but between the different things I'm working on, I literally did that more than 20 times today. It's a very significant increase in time, for an uncertain benefit. What do we gain by eliminating that .git from the provisioned copy of the workspace? If we're really getting something out of eliminating the git metadata from the provisioned vagrant image, then it might be worth the price. But if not, then why are we making work harder? Bill Farner wrote: It's a *lot* slower. Trying it out with a simple change in the client, the round-trip time for a change using git is under five seconds, for git commit, git pull, and pants to build the client. Yup, it's bound to be slower as-is given that there are more builds taking place. Looks like the only way to pass arguments to the provisioner script is via environment variables. How would you feel about that to let you rebuild only specific components? If we're really getting something out of eliminating the git metadata from the provisioned vagrant image, then it might be worth the price. My intent is to standardize on the workflow within the vagrant environment, leaning on vagrant to hide details of copying code and building. Seems like a giant kludge to work around something that isn't a problem. We've got a clean workflow that's fast, easy, and does a good job. Why not just document it, instead of trying to retrofit vagrant provisioning into doing something that it wasn't meant to do? If you really dislike using git for this, why not just write a companion script that does the rsync, instead of kludging the provisioner? - Mark --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22008/#review44255 --- On May 29, 2014, 12:43 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22008/ --- (Updated May 29, 2014, 12:43 a.m.) Review request for Aurora and Mark Chu-Carroll. Repository: aurora Description --- Exclude .git from rsync copy in vagrant. Diffs - examples/vagrant/provision-dev-cluster.sh ce936c19a42f4968d4706e6ef38c25db01ae2c5d Diff: https://reviews.apache.org/r/22008/diff/ Testing --- vagrant up Thanks, Bill Farner
Re: Review Request 22008: Exclude .git from rsync copy in vagrant.
On May 29, 2014, 11:58 a.m., Mark Chu-Carroll wrote: Why? Right now, I do a lot of debugging of the client using vagrant. My typical workflow is: - Make the change in my git workspace. - Commit it to a branch. - vagrant ssh into the virtual cluster - git pull /vagrant mybranch - build client - test If we eliminate that .git file, then the aurora copy in the vagrant host can no longer be used for pulls. We can't just use the /vagrant copy, because that's got a ton of state. With this change, to do a similar dev workflow, I'd need to create a git workspace cloned from /vagrant, and also add new stuff to copy the resulting aurora/aurora2 pexes into the original /vagrant copy. It makes the workflow more complicated, and I'm not clear on what benefits it produces in exchange. Bill Farner wrote: The workflow would be slightly different than you describe. To update builds in the VM, you would run vagrant provision, and rsync will pick up the changes. Does that sound reasonable? Mark Chu-Carroll wrote: It's a *lot* slower. Trying it out with a simple change in the client, the round-trip time for a change using git is under five seconds, for git commit, git pull, and pants to build the client. Running vagrant provision took one minute and 20 seconds for an equivalent process. It's just a minute - but between the different things I'm working on, I literally did that more than 20 times today. It's a very significant increase in time, for an uncertain benefit. What do we gain by eliminating that .git from the provisioned copy of the workspace? If we're really getting something out of eliminating the git metadata from the provisioned vagrant image, then it might be worth the price. But if not, then why are we making work harder? Bill Farner wrote: It's a *lot* slower. Trying it out with a simple change in the client, the round-trip time for a change using git is under five seconds, for git commit, git pull, and pants to build the client. Yup, it's bound to be slower as-is given that there are more builds taking place. Looks like the only way to pass arguments to the provisioner script is via environment variables. How would you feel about that to let you rebuild only specific components? If we're really getting something out of eliminating the git metadata from the provisioned vagrant image, then it might be worth the price. My intent is to standardize on the workflow within the vagrant environment, leaning on vagrant to hide details of copying code and building. Mark Chu-Carroll wrote: Seems like a giant kludge to work around something that isn't a problem. We've got a clean workflow that's fast, easy, and does a good job. Why not just document it, instead of trying to retrofit vagrant provisioning into doing something that it wasn't meant to do? If you really dislike using git for this, why not just write a companion script that does the rsync, instead of kludging the provisioner? Ah, yes - i did think about pushing the rsync command into a script in the default PATH, i'm fine with that. My goal is not to eradicate git, but to simplify use of the vagrant environment. For example, Maxim and David tend to favor ./gradlew run partially because it's one command. Meanwhile i'd like to remove ./gradlew run for a few reasons. I see 'vagrant provision' as the possible replacement for that command. I'm happy to proceed with setting up a command that does the code sync, but i'll be looking for more than that to get to a single command. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22008/#review44255 --- On May 29, 2014, 4:43 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22008/ --- (Updated May 29, 2014, 4:43 a.m.) Review request for Aurora and Mark Chu-Carroll. Repository: aurora Description --- Exclude .git from rsync copy in vagrant. Diffs - examples/vagrant/provision-dev-cluster.sh ce936c19a42f4968d4706e6ef38c25db01ae2c5d Diff: https://reviews.apache.org/r/22008/diff/ Testing --- vagrant up Thanks, Bill Farner
Re: Review Request 22008: Exclude .git from rsync copy in vagrant.
On May 29, 2014, 11:58 a.m., Mark Chu-Carroll wrote: Why? Right now, I do a lot of debugging of the client using vagrant. My typical workflow is: - Make the change in my git workspace. - Commit it to a branch. - vagrant ssh into the virtual cluster - git pull /vagrant mybranch - build client - test If we eliminate that .git file, then the aurora copy in the vagrant host can no longer be used for pulls. We can't just use the /vagrant copy, because that's got a ton of state. With this change, to do a similar dev workflow, I'd need to create a git workspace cloned from /vagrant, and also add new stuff to copy the resulting aurora/aurora2 pexes into the original /vagrant copy. It makes the workflow more complicated, and I'm not clear on what benefits it produces in exchange. Bill Farner wrote: The workflow would be slightly different than you describe. To update builds in the VM, you would run vagrant provision, and rsync will pick up the changes. Does that sound reasonable? Mark Chu-Carroll wrote: It's a *lot* slower. Trying it out with a simple change in the client, the round-trip time for a change using git is under five seconds, for git commit, git pull, and pants to build the client. Running vagrant provision took one minute and 20 seconds for an equivalent process. It's just a minute - but between the different things I'm working on, I literally did that more than 20 times today. It's a very significant increase in time, for an uncertain benefit. What do we gain by eliminating that .git from the provisioned copy of the workspace? If we're really getting something out of eliminating the git metadata from the provisioned vagrant image, then it might be worth the price. But if not, then why are we making work harder? Bill Farner wrote: It's a *lot* slower. Trying it out with a simple change in the client, the round-trip time for a change using git is under five seconds, for git commit, git pull, and pants to build the client. Yup, it's bound to be slower as-is given that there are more builds taking place. Looks like the only way to pass arguments to the provisioner script is via environment variables. How would you feel about that to let you rebuild only specific components? If we're really getting something out of eliminating the git metadata from the provisioned vagrant image, then it might be worth the price. My intent is to standardize on the workflow within the vagrant environment, leaning on vagrant to hide details of copying code and building. Mark Chu-Carroll wrote: Seems like a giant kludge to work around something that isn't a problem. We've got a clean workflow that's fast, easy, and does a good job. Why not just document it, instead of trying to retrofit vagrant provisioning into doing something that it wasn't meant to do? If you really dislike using git for this, why not just write a companion script that does the rsync, instead of kludging the provisioner? Bill Farner wrote: Ah, yes - i did think about pushing the rsync command into a script in the default PATH, i'm fine with that. My goal is not to eradicate git, but to simplify use of the vagrant environment. For example, Maxim and David tend to favor ./gradlew run partially because it's one command. Meanwhile i'd like to remove ./gradlew run for a few reasons. I see 'vagrant provision' as the possible replacement for that command. I'm happy to proceed with setting up a command that does the code sync, but i'll be looking for more than that to get to a single command. Mark Chu-Carroll wrote: I'm good with getting to a single command, so long as it isn't a big step backwards for my productivity. I'm happy to help however I can - just let me know what you need. I see two workable routes, do either of these sound usable? Do any other approaches come to mind? - The environment variable approach. Upside is that this makes vagrant provision the uniform interface. # Piecemeal rebuild COMPONENTS='scheduler,observer' vagrant provision # Rebuild everything vagrant provision - A build script for each component, installed during provisioning. Downside is that piecemeal and full build approaches are very different. # Piecemeal rebuild vagrant ssh -c 'rebuild client,scheduler' # Rebuild everything vagrant provision - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22008/#review44255 --- On May 29, 2014, 4:43 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply,
Re: Review Request 22008: Exclude .git from rsync copy in vagrant.
On May 29, 2014, 7:58 a.m., Mark Chu-Carroll wrote: Why? Right now, I do a lot of debugging of the client using vagrant. My typical workflow is: - Make the change in my git workspace. - Commit it to a branch. - vagrant ssh into the virtual cluster - git pull /vagrant mybranch - build client - test If we eliminate that .git file, then the aurora copy in the vagrant host can no longer be used for pulls. We can't just use the /vagrant copy, because that's got a ton of state. With this change, to do a similar dev workflow, I'd need to create a git workspace cloned from /vagrant, and also add new stuff to copy the resulting aurora/aurora2 pexes into the original /vagrant copy. It makes the workflow more complicated, and I'm not clear on what benefits it produces in exchange. Bill Farner wrote: The workflow would be slightly different than you describe. To update builds in the VM, you would run vagrant provision, and rsync will pick up the changes. Does that sound reasonable? Mark Chu-Carroll wrote: It's a *lot* slower. Trying it out with a simple change in the client, the round-trip time for a change using git is under five seconds, for git commit, git pull, and pants to build the client. Running vagrant provision took one minute and 20 seconds for an equivalent process. It's just a minute - but between the different things I'm working on, I literally did that more than 20 times today. It's a very significant increase in time, for an uncertain benefit. What do we gain by eliminating that .git from the provisioned copy of the workspace? If we're really getting something out of eliminating the git metadata from the provisioned vagrant image, then it might be worth the price. But if not, then why are we making work harder? Bill Farner wrote: It's a *lot* slower. Trying it out with a simple change in the client, the round-trip time for a change using git is under five seconds, for git commit, git pull, and pants to build the client. Yup, it's bound to be slower as-is given that there are more builds taking place. Looks like the only way to pass arguments to the provisioner script is via environment variables. How would you feel about that to let you rebuild only specific components? If we're really getting something out of eliminating the git metadata from the provisioned vagrant image, then it might be worth the price. My intent is to standardize on the workflow within the vagrant environment, leaning on vagrant to hide details of copying code and building. Mark Chu-Carroll wrote: Seems like a giant kludge to work around something that isn't a problem. We've got a clean workflow that's fast, easy, and does a good job. Why not just document it, instead of trying to retrofit vagrant provisioning into doing something that it wasn't meant to do? If you really dislike using git for this, why not just write a companion script that does the rsync, instead of kludging the provisioner? Bill Farner wrote: Ah, yes - i did think about pushing the rsync command into a script in the default PATH, i'm fine with that. My goal is not to eradicate git, but to simplify use of the vagrant environment. For example, Maxim and David tend to favor ./gradlew run partially because it's one command. Meanwhile i'd like to remove ./gradlew run for a few reasons. I see 'vagrant provision' as the possible replacement for that command. I'm happy to proceed with setting up a command that does the code sync, but i'll be looking for more than that to get to a single command. Mark Chu-Carroll wrote: I'm good with getting to a single command, so long as it isn't a big step backwards for my productivity. I'm happy to help however I can - just let me know what you need. Bill Farner wrote: I see two workable routes, do either of these sound usable? Do any other approaches come to mind? - The environment variable approach. Upside is that this makes vagrant provision the uniform interface. # Piecemeal rebuild COMPONENTS='scheduler,observer' vagrant provision # Rebuild everything vagrant provision - A build script for each component, installed during provisioning. Downside is that piecemeal and full build approaches are very different. # Piecemeal rebuild vagrant ssh -c 'rebuild client,scheduler' # Rebuild everything vagrant provision I strongly prefer the latter approach. My experience with using environment variables for this kind of thing is awful - every time I've done it, I've lived to regret it. - Mark --- This is an automatically generated e-mail. To reply,
Re: Review Request 22082: Modify the way that config binding helpers get registered.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22082/#review44394 --- Mark - would you mind adding a committer to the People line? We don't have enforcement on this yet (one of these days i'll free up time to write the git hook), but it seems prudent to always have at least one committer as an explicit reviewer. - Bill Farner On May 30, 2014, 1:57 p.m., Mark Chu-Carroll wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22082/ --- (Updated May 30, 2014, 1:57 p.m.) Review request for Aurora, Antoine Tollenaere and David McLaughlin. Bugs: aurora-496 https://issues.apache.org/jira/browse/aurora-496 Repository: aurora Description --- Modify the way that config binding helpers get registered. Config binding helpers (components that add macros to the pystachio config language) self-registered in a way that made it difficult to provide parameters to initialize them. This change switches to an explicit construction/registration, instead of auto-construction when the class is registered. (Interestingly, this is the way that the documentation on the binding helpers code says that it works!) With this change, instead of writing: FooHelper.register() You write: BindingHelper.register(FooHelper()) Which makes it possible to do: BindingHelper.register(FooHelper(url=bar)) Diffs - src/main/python/apache/aurora/client/binding_helper.py 47448e061c4afd85b88ee3f106f49884e2369e8a Diff: https://reviews.apache.org/r/22082/diff/ Testing --- All client unit tests run and passed. Thanks, Mark Chu-Carroll
Re: Review Request 22082: Modify the way that config binding helpers get registered.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22082/ --- (Updated May 30, 2014, 2:09 p.m.) Review request for Aurora, Antoine Tollenaere, David McLaughlin, and Bill Farner. Changes --- Add a committer to the reviewers. Bugs: aurora-496 https://issues.apache.org/jira/browse/aurora-496 Repository: aurora Description --- Modify the way that config binding helpers get registered. Config binding helpers (components that add macros to the pystachio config language) self-registered in a way that made it difficult to provide parameters to initialize them. This change switches to an explicit construction/registration, instead of auto-construction when the class is registered. (Interestingly, this is the way that the documentation on the binding helpers code says that it works!) With this change, instead of writing: FooHelper.register() You write: BindingHelper.register(FooHelper()) Which makes it possible to do: BindingHelper.register(FooHelper(url=bar)) Diffs - src/main/python/apache/aurora/client/binding_helper.py 47448e061c4afd85b88ee3f106f49884e2369e8a Diff: https://reviews.apache.org/r/22082/diff/ Testing --- All client unit tests run and passed. Thanks, Mark Chu-Carroll
Re: Review Request 22065: Fix /cron endpoint.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22065/#review44396 --- Ship it! Ship It! - David McLaughlin On May 30, 2014, 1:32 a.m., Kevin Sweeney wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22065/ --- (Updated May 30, 2014, 1:32 a.m.) Review request for Aurora, David McLaughlin and Bill Farner. Bugs: AURORA-478 https://issues.apache.org/jira/browse/AURORA-478 Repository: aurora Description --- Fix /cron endpoint. Diffs - src/main/java/org/apache/aurora/scheduler/cron/quartz/CronJobManagerImpl.java 3359425b5f19e68f33c46c5191ad100c1857d978 src/main/java/org/apache/aurora/scheduler/http/Cron.java 6ccf5833012b4282e6a0fc94db39ac4ccd5ce77f src/test/java/org/apache/aurora/scheduler/cron/quartz/CronJobManagerImplTest.java efa0a583dfa63ae1240a2fff128d97497c83536e src/test/java/org/apache/aurora/scheduler/http/CronTest.java PRE-CREATION Diff: https://reviews.apache.org/r/22065/diff/ Testing --- ./gradlew -Pq build ./gradlew run curl -s http://localhost:8081/cron | python -m json.tool Thanks, Kevin Sweeney
Re: Review Request 22082: Modify the way that config binding helpers get registered.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22082/#review44399 --- Ship it! Ship It! - David McLaughlin On May 30, 2014, 6:09 p.m., Mark Chu-Carroll wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22082/ --- (Updated May 30, 2014, 6:09 p.m.) Review request for Aurora, Antoine Tollenaere, David McLaughlin, and Bill Farner. Bugs: aurora-496 https://issues.apache.org/jira/browse/aurora-496 Repository: aurora Description --- Modify the way that config binding helpers get registered. Config binding helpers (components that add macros to the pystachio config language) self-registered in a way that made it difficult to provide parameters to initialize them. This change switches to an explicit construction/registration, instead of auto-construction when the class is registered. (Interestingly, this is the way that the documentation on the binding helpers code says that it works!) With this change, instead of writing: FooHelper.register() You write: BindingHelper.register(FooHelper()) Which makes it possible to do: BindingHelper.register(FooHelper(url=bar)) Diffs - src/main/python/apache/aurora/client/binding_helper.py 47448e061c4afd85b88ee3f106f49884e2369e8a Diff: https://reviews.apache.org/r/22082/diff/ Testing --- All client unit tests run and passed. Thanks, Mark Chu-Carroll
Review Request 22092: Added a comment to .gitignore.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22092/ --- Review request for Aurora and Bill Farner. Bugs: AURORA-485 https://issues.apache.org/jira/browse/AURORA-485 Repository: aurora Description --- Added a comment to .gitignore. Diffs - .gitignore bed5b8b8a02e957e05b8bcc8fed925f1432973cf Diff: https://reviews.apache.org/r/22092/diff/ Testing --- Thanks, Suman Karumuri
Re: Review Request 22032: first draft of a developing client v2 document.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22032/#review44415 --- docs/developing-aurora-client.md https://reviews.apache.org/r/22032/#comment78793 The executor is an important part of aurora - but how is it part of the client? The executor is entirely a cluster-side thing running on the slaves. docs/developing-aurora-client.md https://reviews.apache.org/r/22032/#comment78794 I think it's worth being complete here. - Mark Chu-Carroll On May 29, 2014, 3:30 p.m., Mark Chu-Carroll wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22032/ --- (Updated May 29, 2014, 3:30 p.m.) Review request for Aurora, David McLaughlin and Henry Saputra. Bugs: aurora-20 https://issues.apache.org/jira/browse/aurora-20 Repository: aurora Description --- first draft of a developing client v2 document. Diffs - docs/developing-aurora-client.md PRE-CREATION Diff: https://reviews.apache.org/r/22032/diff/ Testing --- n/a Thanks, Mark Chu-Carroll
Re: Review Request 22032: first draft of a developing client v2 document.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22032/ --- (Updated May 30, 2014, 3:40 p.m.) Review request for Aurora, David McLaughlin and Henry Saputra. Changes --- Address reviews. Bugs: aurora-20 https://issues.apache.org/jira/browse/aurora-20 Repository: aurora Description --- first draft of a developing client v2 document. Diffs (updated) - docs/developing-aurora-client.md PRE-CREATION Diff: https://reviews.apache.org/r/22032/diff/ Testing --- n/a Thanks, Mark Chu-Carroll
Re: Review Request 22092: Added a comment to .gitignore.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22092/#review44417 --- It wasn't obvious, but in the last review i was subtly trying to nudge you to reevaluate all the directory entries in here. Seems like they all deserve the same treatment. - Bill Farner On May 30, 2014, 7:31 p.m., Suman Karumuri wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22092/ --- (Updated May 30, 2014, 7:31 p.m.) Review request for Aurora and Bill Farner. Bugs: AURORA-485 https://issues.apache.org/jira/browse/AURORA-485 Repository: aurora Description --- Added a comment to .gitignore. Diffs - .gitignore bed5b8b8a02e957e05b8bcc8fed925f1432973cf Diff: https://reviews.apache.org/r/22092/diff/ Testing --- Thanks, Suman Karumuri
Re: Review Request 22032: first draft of a developing client v2 document.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22032/ --- (Updated May 30, 2014, 4:14 p.m.) Review request for Aurora, David McLaughlin and Henry Saputra. Changes --- More changes. Bugs: aurora-20 https://issues.apache.org/jira/browse/aurora-20 Repository: aurora Description --- first draft of a developing client v2 document. Diffs (updated) - docs/developing-aurora-client.md PRE-CREATION Diff: https://reviews.apache.org/r/22032/diff/ Testing --- n/a Thanks, Mark Chu-Carroll
Re: Review Request 22094: Updating vagrant cleanup steps.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22094/ --- (Updated May 30, 2014, 9:08 p.m.) Review request for Aurora and Bill Farner. Changes --- CR comments. Bugs: AURORA-499 https://issues.apache.org/jira/browse/AURORA-499 Repository: aurora Description --- Updating vagrant cleanup steps. Diffs (updated) - docs/vagrant.md 6c0876dd5a215b93c7894c3d5299cc65fd51d192 Diff: https://reviews.apache.org/r/22094/diff/ Testing --- Thanks, Maxim Khutornenko
Re: Review Request 22094: Updating vagrant cleanup steps.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22094/ --- (Updated May 30, 2014, 9:13 p.m.) Review request for Aurora and Bill Farner. Changes --- CR comments. Bugs: AURORA-499 https://issues.apache.org/jira/browse/AURORA-499 Repository: aurora Description --- Updating vagrant cleanup steps. Diffs (updated) - docs/vagrant.md 6c0876dd5a215b93c7894c3d5299cc65fd51d192 Diff: https://reviews.apache.org/r/22094/diff/ Testing --- Thanks, Maxim Khutornenko
Re: Review Request 22008: Exclude .git from rsync copy in vagrant.
On May 29, 2014, 11:58 a.m., Mark Chu-Carroll wrote: Why? Right now, I do a lot of debugging of the client using vagrant. My typical workflow is: - Make the change in my git workspace. - Commit it to a branch. - vagrant ssh into the virtual cluster - git pull /vagrant mybranch - build client - test If we eliminate that .git file, then the aurora copy in the vagrant host can no longer be used for pulls. We can't just use the /vagrant copy, because that's got a ton of state. With this change, to do a similar dev workflow, I'd need to create a git workspace cloned from /vagrant, and also add new stuff to copy the resulting aurora/aurora2 pexes into the original /vagrant copy. It makes the workflow more complicated, and I'm not clear on what benefits it produces in exchange. Bill Farner wrote: The workflow would be slightly different than you describe. To update builds in the VM, you would run vagrant provision, and rsync will pick up the changes. Does that sound reasonable? Mark Chu-Carroll wrote: It's a *lot* slower. Trying it out with a simple change in the client, the round-trip time for a change using git is under five seconds, for git commit, git pull, and pants to build the client. Running vagrant provision took one minute and 20 seconds for an equivalent process. It's just a minute - but between the different things I'm working on, I literally did that more than 20 times today. It's a very significant increase in time, for an uncertain benefit. What do we gain by eliminating that .git from the provisioned copy of the workspace? If we're really getting something out of eliminating the git metadata from the provisioned vagrant image, then it might be worth the price. But if not, then why are we making work harder? Bill Farner wrote: It's a *lot* slower. Trying it out with a simple change in the client, the round-trip time for a change using git is under five seconds, for git commit, git pull, and pants to build the client. Yup, it's bound to be slower as-is given that there are more builds taking place. Looks like the only way to pass arguments to the provisioner script is via environment variables. How would you feel about that to let you rebuild only specific components? If we're really getting something out of eliminating the git metadata from the provisioned vagrant image, then it might be worth the price. My intent is to standardize on the workflow within the vagrant environment, leaning on vagrant to hide details of copying code and building. Mark Chu-Carroll wrote: Seems like a giant kludge to work around something that isn't a problem. We've got a clean workflow that's fast, easy, and does a good job. Why not just document it, instead of trying to retrofit vagrant provisioning into doing something that it wasn't meant to do? If you really dislike using git for this, why not just write a companion script that does the rsync, instead of kludging the provisioner? Bill Farner wrote: Ah, yes - i did think about pushing the rsync command into a script in the default PATH, i'm fine with that. My goal is not to eradicate git, but to simplify use of the vagrant environment. For example, Maxim and David tend to favor ./gradlew run partially because it's one command. Meanwhile i'd like to remove ./gradlew run for a few reasons. I see 'vagrant provision' as the possible replacement for that command. I'm happy to proceed with setting up a command that does the code sync, but i'll be looking for more than that to get to a single command. Mark Chu-Carroll wrote: I'm good with getting to a single command, so long as it isn't a big step backwards for my productivity. I'm happy to help however I can - just let me know what you need. Bill Farner wrote: I see two workable routes, do either of these sound usable? Do any other approaches come to mind? - The environment variable approach. Upside is that this makes vagrant provision the uniform interface. # Piecemeal rebuild COMPONENTS='scheduler,observer' vagrant provision # Rebuild everything vagrant provision - A build script for each component, installed during provisioning. Downside is that piecemeal and full build approaches are very different. # Piecemeal rebuild vagrant ssh -c 'rebuild client,scheduler' # Rebuild everything vagrant provision Mark Chu-Carroll wrote: I strongly prefer the latter approach. My experience with using environment variables for this kind of thing is awful - every time I've done it, I've lived to regret it. How about a set of component specific provisioning scripts? That would let us do
Re: Review Request 21440: Implementing parallel updater
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21440/ --- (Updated May 30, 2014, 10:08 p.m.) Review request for Aurora, Mark Chu-Carroll and Brian Wickman. Changes --- Rebased. Brian, ping. Bugs: AURORA-350 https://issues.apache.org/jira/browse/AURORA-350 Repository: aurora Description --- The updater now spawns upto batch_size threads to process one instance per thread. All mutating calls are multiplexed by the SchedulerMux to do batch kill/add/restart calls. This is the first step towards a fully multiplexed SchedulerProxy and is intended to mitigate LDAP/scheduler load. Diffs (updated) - src/main/python/apache/aurora/client/api/instance_watcher.py e09aa9a6c32c17f13c9b8ff3a589919587bd839b src/main/python/apache/aurora/client/api/job_monitor.py d176995fca68d42fcc2d3989483eaf520d0d737f src/main/python/apache/aurora/client/api/scheduler_client.py 7be974eb91089f776656ce65b64ee6d8c5b46394 src/main/python/apache/aurora/client/api/updater.py ea7285a75020a47142e1761c7ed455cdc838e37c src/main/python/apache/aurora/client/api/updater_util.py 04105de8fb2ce1cab049eb06fd313a43bdcd28db src/test/python/apache/aurora/client/api/test_instance_watcher.py b2d0c804ae2b2095d8d2a99ea42f4da06041cec8 src/test/python/apache/aurora/client/api/test_job_monitor.py 665db74475f4828af2050e98e20bbb3b1b29cf0c src/test/python/apache/aurora/client/api/test_updater.py ba783da7c0d93bb0bfd03809f62ddcad3f98cd0a src/test/python/apache/aurora/client/cli/test_create.py b186b52416a2fae8de28fd1d21e7eec07fea8e55 src/test/python/apache/aurora/client/cli/test_kill.py 666ec3aa0745191aa1395e47728343cd0eda7115 src/test/python/apache/aurora/client/cli/test_restart.py 50acc09491ac21935af78499ad66726df5a8f2ff src/test/python/apache/aurora/client/cli/test_update.py a2abc5eb0f11f9bc563f4504c93fcf5b7520d141 src/test/python/apache/aurora/client/cli/util.py dac4928111200136a9987c9622087e8cdca7f2d2 src/test/python/apache/aurora/client/commands/test_create.py 75f068250b31b656c9c87a6aa66872fbb777b0c0 src/test/python/apache/aurora/client/commands/test_kill.py 3e2ac1fcea301f0ae986b61d9851d10e86996a20 src/test/python/apache/aurora/client/commands/test_restart.py 6e0159f134388a251cb44cd700102d05467a9062 src/test/python/apache/aurora/client/commands/test_update.py c5afbd33d1b2f82e9603c93b967fbc942c0952d7 src/test/python/apache/aurora/client/commands/util.py 84784171816797f3a4fa4c5238d19b626e68ff44 src/test/python/apache/aurora/client/fake_scheduler_proxy.py 2a4773c81efb390385f675854e9631500b263a45 src/test/sh/org/apache/aurora/e2e/http/http_example.aurora 80871273fc4d47558253e6b09c92724e8693bc11 src/test/sh/org/apache/aurora/e2e/http/http_example_updated.aurora fc723cf232ddbc10458fc394e37358c8523118c2 src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 9c5652829ac306dda5f7e95e164c85713e18988f src/test/sh/org/apache/aurora/e2e/test_end_to_end_v2.sh 2af256d65850bd86279dff4b5c53f234cf7a Diff: https://reviews.apache.org/r/21440/diff/ Testing --- ./pants src/test/python:all src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh Thanks, Maxim Khutornenko
Review Request 22097: Remove unused thrift flags from scheduler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22097/ --- Review request for Aurora, David McLaughlin and Maxim Khutornenko. Repository: aurora Description --- Remove unused thrift flags from scheduler Diffs - src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 321ac3a4f70d374ec794e577d7ac7fb013ca1c20 src/main/java/org/apache/aurora/scheduler/thrift/ThriftConfiguration.java 6b28fa623d657a198b428ce4bb3bbeae2db461e0 src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 5e12e4ecd2029284ca864e041457d67364d4 src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java aad18b484c80cc8e8270fa67af9cdaaa10460a9c Diff: https://reviews.apache.org/r/22097/diff/ Testing --- ./gradlew -Pq build Thanks, Kevin Sweeney
Re: Review Request 22032: first draft of a developing client v2 document.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22032/#review44436 --- Ship it! Ship It! - Kevin Sweeney On May 30, 2014, 1:14 p.m., Mark Chu-Carroll wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22032/ --- (Updated May 30, 2014, 1:14 p.m.) Review request for Aurora, David McLaughlin and Henry Saputra. Bugs: aurora-20 https://issues.apache.org/jira/browse/aurora-20 Repository: aurora Description --- first draft of a developing client v2 document. Diffs - docs/developing-aurora-client.md PRE-CREATION Diff: https://reviews.apache.org/r/22032/diff/ Testing --- n/a Thanks, Mark Chu-Carroll
