{"action":"created","issue":{"url":"https://api.github.com/repos/broadinstitute/rawls/issues/464","repository_url":"https://api.github.com/repos/broadinstitute/rawls","labels_url":"https://api.github.com/repos/broadinstitute/rawls/issues/464/labels{/name}","comments_url":"https://api.github.com/repos/broadinstitute/rawls/issues/464/comments","events_url":"https://api.github.com/repos/broadinstitute/rawls/issues/464/events","html_url":"https://github.com/broadinstitute/rawls/pull/464","id":160484747,"number":464,"title":"[DNM] arch review, please: sharding workflows to Cromwell","user":{"login":"davidangb","id":6041577,"avatar_url":"https://avatars.githubusercontent.com/u/6041577?v=3","gravatar_id":"","url":"https://api.github.com/users/davidangb","html_url":"https://github.com/davidangb","followers_url":"https://api.github.com/users/davidangb/followers","following_url":"https://api.github.com/users/davidangb/following{/other_user}","gists_url":"https://api.github.com/users/davidangb/gists{/gist_id}","starred_url":"https://api.github.com/users/davidangb/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/davidangb/subscriptions","organizations_url":"https://api.github.com/users/davidangb/orgs","repos_url":"https://api.github.com/users/davidangb/repos","events_url":"https://api.github.com/users/davidangb/events{/privacy}","received_events_url":"https://api.github.com/users/davidangb/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":0,"created_at":"2016-06-15T17:56:21Z","updated_at":"2016-06-15T18:01:36Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/broadinstitute/rawls/pulls/464","html_url":"https://github.com/broadinstitute/rawls/pull/464","diff_url":"https://github.com/broadinstitute/rawls/pull/464.diff","patch_url":"https://github.com/broadinstitute/rawls/pull/464.patch"},"body":"do not merge.\r\n\r\nLooking for high-level architectural feedback on this approach to sharding workflows to Cromwell. Please don't spend too much time on syntax, as this may get refactored.\r\n\r\nFunctionally, I want to have an algorithm to route batches of workflows to a cromwell instance. Once submitted, I'd write the cromwell instance into the workflow's db row. Anything that queries cromwell about that workflow would read the cromwell instance from the Workflow or WorkflowRecord object, and route to that instance.\r\n\r\nThe basics of my approach in code are:\r\n\r\n1. In Boot.scala, create an ExecutionServiceCluster object containing all the cromwell instances from config.\r\n2. Anywhere we previously passed an ExecutionServiceDAO into the constructor for an Actor or monitor or whatever, we now pass an ExecutionServiceCluster\r\n3. Anywhere we previously called a method on ExecutionServiceDAO, we now ask ExecutionServiceCluster which cromwell to use, retrieve that cromwell's DAO, and call the method on that DAO.\r\n\r\nStill a whole lot left to do:\r\n* actually implement the sharding algorithm, including persisting the cromwell instance onto a workflow once it's been submitted\r\n* add a spate of tests\r\n* fix the four compilation errors in tests\r\n* have a more graceful way to create ExecutionServiceCluster in tests; the test code changes are messy.\r\n\r\n\r\n\r\n- [ ] **Submitter**: Rebase to develop. DO NOT SQUASH\r\n- [ ] **Submitter**: Make sure Swagger is updated if API changes\r\n- [ ] **Submitter**: Make sure documentation for code is complete\r\n- [ ] **Submitter**: Make sure liquibase is updated if appropriate\r\n- [ ] **Submitter**: Review code comments; remove done TODOs, create stories for remaining TODOs\r\n- [ ] **Submitter**: Include the JIRA issue number in the PR description\r\n- [ ] **Submitter**: Add description or comments on the PR explaining the hows/whys (if not obvious)\r\n- [ ] Tell ![](http://i.imgur.com/9dLzbPd.png) that the PR exists if he wants to look at it **(apply requires_doge label)**\r\n- [ ] Anoint a lead reviewer (LR). **Assign PR to LR**\r\n- [ ] **LR**: Initial review by LR and others.\r\n- [ ] Comment / review / update cycle:\r\n  * Rest of team may comments on PR at will\r\n  * **LR assigns to submitter** for feedback fixes\r\n  * Submitter updates documentation as needed\r\n  * Submitter rebases to develop again if necessary\r\n  * Submitter makes further commits. DO NOT SQUASH. **Reassign to LR** for further feedback\r\n- [ ] ![](http://i.imgur.com/9dLzbPd.png) sign off\r\n- [ ] **LR** sign off\r\n- [ ] **Assign to submitter** to finalize\r\n- [ ] **Submitter**: Squash commits, rebase if necessary\r\n- [ ] **Submitter**: Verify all tests go green, including CI tests\r\n- [ ] **Submitter**: Merge to develop \r\n- [ ] **Submitter**: Delete branch after merge\r\n- [ ] **Submitter**: Check configuration files in Jenkins in case they need changes\r\n- [ ] **Submitter**: **Test this change works on dev environment after deployment**. YOU own getting it fixed if dev isn't working for ANY reason!\r\n- [ ] **Submitter**: Verify swagger UI on dev environment still works after deployment\r\n- [ ] **Submitter**: Inform other teams of any API changes via hipchat and/or email\r\n- [ ] **Submitter**: Mark JIRA issue as resolved once this checklist is completed\r\n\r\n"},"comment":{"url":"https://api.github.com/repos/broadinstitute/rawls/issues/comments/226269396","html_url":"https://github.com/broadinstitute/rawls/pull/464#issuecomment-226269396","issue_url":"https://api.github.com/repos/broadinstitute/rawls/issues/464","id":226269396,"user":{"login":"helgridly","id":775136,"avatar_url":"https://avatars.githubusercontent.com/u/775136?v=3","gravatar_id":"","url":"https://api.github.com/users/helgridly","html_url":"https://github.com/helgridly","followers_url":"https://api.github.com/users/helgridly/followers","following_url":"https://api.github.com/users/helgridly/following{/other_user}","gists_url":"https://api.github.com/users/helgridly/gists{/gist_id}","starred_url":"https://api.github.com/users/helgridly/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/helgridly/subscriptions","organizations_url":"https://api.github.com/users/helgridly/orgs","repos_url":"https://api.github.com/users/helgridly/repos","events_url":"https://api.github.com/users/helgridly/events{/privacy}","received_events_url":"https://api.github.com/users/helgridly/received_events","type":"User","site_admin":false},"created_at":"2016-06-15T18:01:36Z","updated_at":"2016-06-15T18:01:36Z","body":"This looks like it does some of the work I was intending to cover in my PR as a result of talking to Doug, so we should probably sync up on this.\r\n\r\nI don't like the pattern of \"ask the cluster for the appropriate DAO and then call the relevant function on the DAO\". Just send the call to submit, status, whatever to the cluster and have it act as a facade. It can return both the workflow ID and some identifier for \"which cromwell\" when submitting, and the caller can be responsible for saving that \"which cromwell\" to the workflow row."}}
{"actor":{"display_login":"helgridly"}}