{"action":"created","issue":{"url":"https://api.github.com/repos/stratis-storage/stratisd/issues/134","repository_url":"https://api.github.com/repos/stratis-storage/stratisd","labels_url":"https://api.github.com/repos/stratis-storage/stratisd/issues/134/labels{/name}","comments_url":"https://api.github.com/repos/stratis-storage/stratisd/issues/134/comments","events_url":"https://api.github.com/repos/stratis-storage/stratisd/issues/134/events","html_url":"https://github.com/stratis-storage/stratisd/pull/134","id":188272046,"number":134,"title":"Minor fixes to sim engine behaviors","user":{"login":"mulkieran","id":1218366,"avatar_url":"https://avatars.githubusercontent.com/u/1218366?v=3","gravatar_id":"","url":"https://api.github.com/users/mulkieran","html_url":"https://github.com/mulkieran","followers_url":"https://api.github.com/users/mulkieran/followers","following_url":"https://api.github.com/users/mulkieran/following{/other_user}","gists_url":"https://api.github.com/users/mulkieran/gists{/gist_id}","starred_url":"https://api.github.com/users/mulkieran/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mulkieran/subscriptions","organizations_url":"https://api.github.com/users/mulkieran/orgs","repos_url":"https://api.github.com/users/mulkieran/repos","events_url":"https://api.github.com/users/mulkieran/events{/privacy}","received_events_url":"https://api.github.com/users/mulkieran/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":2,"created_at":"2016-11-09T15:17:09Z","updated_at":"2016-11-14T14:08:39Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/stratis-storage/stratisd/pulls/134","html_url":"https://github.com/stratis-storage/stratisd/pull/134","diff_url":"https://github.com/stratis-storage/stratisd/pull/134.diff","patch_url":"https://github.com/stratis-storage/stratisd/pull/134.patch"},"body":""},"comment":{"url":"https://api.github.com/repos/stratis-storage/stratisd/issues/comments/260343612","html_url":"https://github.com/stratis-storage/stratisd/pull/134#issuecomment-260343612","issue_url":"https://api.github.com/repos/stratis-storage/stratisd/issues/134","id":260343612,"user":{"login":"mulkieran","id":1218366,"avatar_url":"https://avatars.githubusercontent.com/u/1218366?v=3","gravatar_id":"","url":"https://api.github.com/users/mulkieran","html_url":"https://github.com/mulkieran","followers_url":"https://api.github.com/users/mulkieran/followers","following_url":"https://api.github.com/users/mulkieran/following{/other_user}","gists_url":"https://api.github.com/users/mulkieran/gists{/gist_id}","starred_url":"https://api.github.com/users/mulkieran/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mulkieran/subscriptions","organizations_url":"https://api.github.com/users/mulkieran/orgs","repos_url":"https://api.github.com/users/mulkieran/repos","events_url":"https://api.github.com/users/mulkieran/events{/privacy}","received_events_url":"https://api.github.com/users/mulkieran/received_events","type":"User","site_admin":false},"created_at":"2016-11-14T14:08:39Z","updated_at":"2016-11-14T14:08:39Z","body":"@agrover I don't think the method should return an error when pool not found.\r\n\r\nOne way to think of this method is to think that it has to do some work, namely some sort of removal. If it is not vouchsafed the opportunity to do this work, execute certain instructions, it should fail. I believe that that view causes all sorts of problems.\r\n\r\nThe second point of view is that the method needs to achieve something and that it should guarantee that it can do it so long as certain conditions are satisfied. This is the notion of pre- and post-conditions, which has a lot of other names like \"Design by Contract\". Here the post-condition of the ```destroy_pool``` method is that the pool should be destroyed. If the pool is not even there, the post-condition is immediately achieved, so the method returns Ok(()). I believe that this is a more helpful view.\r\n\r\nWhat sort of problems does the first view cause? Problems with reasoning and failures of code. An obvious problem is if for some reason two requests to destroy the pool come in from two entities, for each of which the pool exists at the time the instructions was issued.. The first one will succeed, but with your recommended construction the second will fail, and all this just depends on who gets in first. With mine, both will get what they wanted without any extra noise. This is good. With your suggested approach, the client code has to look out for ```POOL_NOTFOUND``` error, but has no means of interpreting its meaning. It could be that it got in there late and some other instruction had already removed the pool, it could be there is a bug in the stratis code (the most likely first suspicion), it could be a mis-spelling in the name.\r\n\r\nI'm not saying that it is wrong to write code that returns an error if the pool turns out not to exist when the command is sent. The CLI can do that somehow, if desired. But the ```destroy_pool``` method is the building block on which all sorts of other code can and will be built. So, it must have the simplest and clearest interface that it can have. I think that the pre-condition/post-condition approach is the best available for achieving this goal."}}
{"actor":{"display_login":"mulkieran"}}