{"action":"created","comment":{"url":"https://api.github.com/repos/simpleenergy/env-excavator/pulls/comments/49884133","id":49884133,"diff_hunk":"@@ -78,8 +83,13 @@ def env_int(name, required=False, default=empty):\n     environment, a `TypeError` will be raised since we don't want to guess\n     about a sensible default.\n     \"\"\"\n-    value = get_env_value(name, required=required, default=default)\n+    if required and default is not empty:\n+        raise ValueError(\"Using `default` with `required=True` is invalid\")\n+\n+    value = get_env_value(name, required=required, default=empty)\n     if value is empty:\n+        if default is not empty:\n+            return default\n         raise ValueError(","path":"excavator/utils.py","position":27,"original_position":27,"commit_id":"848120125c6cd74698718b19954fe57461e1def5","original_commit_id":"848120125c6cd74698718b19954fe57461e1def5","user":{"login":"wlonk","id":925990,"avatar_url":"https://avatars.githubusercontent.com/u/925990?v=3","gravatar_id":"","url":"https://api.github.com/users/wlonk","html_url":"https://github.com/wlonk","followers_url":"https://api.github.com/users/wlonk/followers","following_url":"https://api.github.com/users/wlonk/following{/other_user}","gists_url":"https://api.github.com/users/wlonk/gists{/gist_id}","starred_url":"https://api.github.com/users/wlonk/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/wlonk/subscriptions","organizations_url":"https://api.github.com/users/wlonk/orgs","repos_url":"https://api.github.com/users/wlonk/repos","events_url":"https://api.github.com/users/wlonk/events{/privacy}","received_events_url":"https://api.github.com/users/wlonk/received_events","type":"User","site_admin":false},"body":"This whole thing is a perfect time for exception handling. Something like the following would make the control flow clearer and cut down on repetition:\r\n\r\n```python\r\ndef env_int(name, required=False, default=empty):\r\n    \"\"\"\r\n    Return an integer derived from an environmental variable.  In the case\r\n    where no default is specified and the variable is not present in the\r\n    environment, a `TypeError` will be raised since we don't want to guess\r\n    about a sensible default.\r\n    \"\"\"\r\n    value = get_env_value(name, required=required, default=default)\r\n    if value is empty:\r\n        raise ValueError(\r\n            \"`env_int` requires either a default value to be specified, or for \"\r\n            \"the variable to be present in the environment\"\r\n        )\r\n    try:\r\n        return int(value)\r\n    except TypeError:\r\n        # We have a non-int-able default, let's just return it.\r\n        return value\r\n```","created_at":"2016-01-15T18:03:53Z","updated_at":"2016-01-15T18:03:53Z","html_url":"https://github.com/simpleenergy/env-excavator/pull/6#discussion_r49884133","pull_request_url":"https://api.github.com/repos/simpleenergy/env-excavator/pulls/6","_links":{"self":{"href":"https://api.github.com/repos/simpleenergy/env-excavator/pulls/comments/49884133"},"html":{"href":"https://github.com/simpleenergy/env-excavator/pull/6#discussion_r49884133"},"pull_request":{"href":"https://api.github.com/repos/simpleenergy/env-excavator/pulls/6"}}},"pull_request":{"url":"https://api.github.com/repos/simpleenergy/env-excavator/pulls/6","id":31956838,"html_url":"https://github.com/simpleenergy/env-excavator/pull/6","diff_url":"https://github.com/simpleenergy/env-excavator/pull/6.diff","patch_url":"https://github.com/simpleenergy/env-excavator/pull/6.patch","issue_url":"https://api.github.com/repos/simpleenergy/env-excavator/issues/6","number":6,"state":"open","locked":false,"title":"Piper/fix env int default handling","user":{"login":"pipermerriam","id":824194,"avatar_url":"https://avatars.githubusercontent.com/u/824194?v=3","gravatar_id":"","url":"https://api.github.com/users/pipermerriam","html_url":"https://github.com/pipermerriam","followers_url":"https://api.github.com/users/pipermerriam/followers","following_url":"https://api.github.com/users/pipermerriam/following{/other_user}","gists_url":"https://api.github.com/users/pipermerriam/gists{/gist_id}","starred_url":"https://api.github.com/users/pipermerriam/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/pipermerriam/subscriptions","organizations_url":"https://api.github.com/users/pipermerriam/orgs","repos_url":"https://api.github.com/users/pipermerriam/repos","events_url":"https://api.github.com/users/pipermerriam/events{/privacy}","received_events_url":"https://api.github.com/users/pipermerriam/received_events","type":"User","site_admin":false},"body":"### What is the problem / feature ?\r\n\r\nBoth `env_int` and `env_list` are written in such a way that passing in a `default` value that isn't able to be processed the same way as the string returned from the environment causes an error.  This isn't good since it's nice to be able to do `excavator.env_int('SOME_VAR', default=None)`.\r\n\r\n### How did it get fixed / implemented ?\r\n\r\nChanged both to return the `default` value unmodified.\r\n\r\n### How can someone test / see it ?\r\n\r\nSee that `env_list` and `env_int` don't muck with the value passed into the `default` keyword.\r\n\r\n*Here is a cute animal picture for your troubles...*\r\n\r\n![goatie](https://cloud.githubusercontent.com/assets/824194/6833337/cffd8dae-d2f1-11e4-8c13-f5dec7bfa410.jpg)\r\n","created_at":"2015-03-25T19:23:23Z","updated_at":"2016-01-15T18:03:53Z","closed_at":null,"merged_at":null,"merge_commit_sha":"018a1942b0cf5100244603e28910880923d9fbe2","assignee":null,"milestone":null,"commits_url":"https://api.github.com/repos/simpleenergy/env-excavator/pulls/6/commits","review_comments_url":"https://api.github.com/repos/simpleenergy/env-excavator/pulls/6/comments","review_comment_url":"https://api.github.com/repos/simpleenergy/env-excavator/pulls/comments{/number}","comments_url":"https://api.github.com/repos/simpleenergy/env-excavator/issues/6/comments","statuses_url":"https://api.github.com/repos/simpleenergy/env-excavator/statuses/848120125c6cd74698718b19954fe57461e1def5","head":{"label":"pipermerriam:piper/fix-env_int-default-handling","ref":"piper/fix-env_int-default-handling","sha":"848120125c6cd74698718b19954fe57461e1def5","user":{"login":"pipermerriam","id":824194,"avatar_url":"https://avatars.githubusercontent.com/u/824194?v=3","gravatar_id":"","url":"https://api.github.com/users/pipermerriam","html_url":"https://github.com/pipermerriam","followers_url":"https://api.github.com/users/pipermerriam/followers","following_url":"https://api.github.com/users/pipermerriam/following{/other_user}","gists_url":"https://api.github.com/users/pipermerriam/gists{/gist_id}","starred_url":"https://api.github.com/users/pipermerriam/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/pipermerriam/subscriptions","organizations_url":"https://api.github.com/users/pipermerriam/orgs","repos_url":"https://api.github.com/users/pipermerriam/repos","events_url":"https://api.github.com/users/pipermerriam/events{/privacy}","received_events_url":"https://api.github.com/users/pipermerriam/received_events","type":"User","site_admin":false},"repo":{"id":28215706,"name":"env-excavator","full_name":"pipermerriam/env-excavator","owner":{"login":"pipermerriam","id":824194,"avatar_url":"https://avatars.githubusercontent.com/u/824194?v=3","gravatar_id":"","url":"https://api.github.com/users/pipermerriam","html_url":"https://github.com/pipermerriam","followers_url":"https://api.github.com/users/pipermerriam/followers","following_url":"https://api.github.com/users/pipermerriam/following{/other_user}","gists_url":"https://api.github.com/users/pipermerriam/gists{/gist_id}","starred_url":"https://api.github.com/users/pipermerriam/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/pipermerriam/subscriptions","organizations_url":"https://api.github.com/users/pipermerriam/orgs","repos_url":"https://api.github.com/users/pipermerriam/repos","events_url":"https://api.github.com/users/pipermerriam/events{/privacy}","received_events_url":"https://api.github.com/users/pipermerriam/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/pipermerriam/env-excavator","description":"","fork":true,"url":"https://api.github.com/repos/pipermerriam/env-excavator","forks_url":"https://api.github.com/repos/pipermerriam/env-excavator/forks","keys_url":"https://api.github.com/repos/pipermerriam/env-excavator/keys{/key_id}","collaborators_url":"https://api.github.com/repos/pipermerriam/env-excavator/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/pipermerriam/env-excavator/teams","hooks_url":"https://api.github.com/repos/pipermerriam/env-excavator/hooks","issue_events_url":"https://api.github.com/repos/pipermerriam/env-excavator/issues/events{/number}","events_url":"https://api.github.com/repos/pipermerriam/env-excavator/events","assignees_url":"https://api.github.com/repos/pipermerriam/env-excavator/assignees{/user}","branches_url":"https://api.github.com/repos/pipermerriam/env-excavator/branches{/branch}","tags_url":"https://api.github.com/repos/pipermerriam/env-excavator/tags","blobs_url":"https://api.github.com/repos/pipermerriam/env-excavator/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/pipermerriam/env-excavator/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/pipermerriam/env-excavator/git/refs{/sha}","trees_url":"https://api.github.com/repos/pipermerriam/env-excavator/git/trees{/sha}","statuses_url":"https://api.github.com/repos/pipermerriam/env-excavator/statuses/{sha}","languages_url":"https://api.github.com/repos/pipermerriam/env-excavator/languages","stargazers_url":"https://api.github.com/repos/pipermerriam/env-excavator/stargazers","contributors_url":"https://api.github.com/repos/pipermerriam/env-excavator/contributors","subscribers_url":"https://api.github.com/repos/pipermerriam/env-excavator/subscribers","subscription_url":"https://api.github.com/repos/pipermerriam/env-excavator/subscription","commits_url":"https://api.github.com/repos/pipermerriam/env-excavator/commits{/sha}","git_commits_url":"https://api.github.com/repos/pipermerriam/env-excavator/git/commits{/sha}","comments_url":"https://api.github.com/repos/pipermerriam/env-excavator/comments{/number}","issue_comment_url":"https://api.github.com/repos/pipermerriam/env-excavator/issues/comments{/number}","contents_url":"https://api.github.com/repos/pipermerriam/env-excavator/contents/{+path}","compare_url":"https://api.github.com/repos/pipermerriam/env-excavator/compare/{base}...{head}","merges_url":"https://api.github.com/repos/pipermerriam/env-excavator/merges","archive_url":"https://api.github.com/repos/pipermerriam/env-excavator/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/pipermerriam/env-excavator/downloads","issues_url":"https://api.github.com/repos/pipermerriam/env-excavator/issues{/number}","pulls_url":"https://api.github.com/repos/pipermerriam/env-excavator/pulls{/number}","milestones_url":"https://api.github.com/repos/pipermerriam/env-excavator/milestones{/number}","notifications_url":"https://api.github.com/repos/pipermerriam/env-excavator/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/pipermerriam/env-excavator/labels{/name}","releases_url":"https://api.github.com/repos/pipermerriam/env-excavator/releases{/id}","created_at":"2014-12-19T05:27:24Z","updated_at":"2015-03-12T02:10:32Z","pushed_at":"2016-01-15T07:48:42Z","git_url":"git://github.com/pipermerriam/env-excavator.git","ssh_url":"git@github.com:pipermerriam/env-excavator.git","clone_url":"https://github.com/pipermerriam/env-excavator.git","svn_url":"https://github.com/pipermerriam/env-excavator","homepage":null,"size":179,"stargazers_count":0,"watchers_count":0,"language":"Python","has_issues":false,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":0,"mirror_url":null,"open_issues_count":0,"forks":0,"open_issues":0,"watchers":0,"default_branch":"master"}},"base":{"label":"simpleenergy:master","ref":"master","sha":"f1ee80798d710ff7ac2e6b3c7a6448d4fdd47568","user":{"login":"simpleenergy","id":522113,"avatar_url":"https://avatars.githubusercontent.com/u/522113?v=3","gravatar_id":"","url":"https://api.github.com/users/simpleenergy","html_url":"https://github.com/simpleenergy","followers_url":"https://api.github.com/users/simpleenergy/followers","following_url":"https://api.github.com/users/simpleenergy/following{/other_user}","gists_url":"https://api.github.com/users/simpleenergy/gists{/gist_id}","starred_url":"https://api.github.com/users/simpleenergy/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/simpleenergy/subscriptions","organizations_url":"https://api.github.com/users/simpleenergy/orgs","repos_url":"https://api.github.com/users/simpleenergy/repos","events_url":"https://api.github.com/users/simpleenergy/events{/privacy}","received_events_url":"https://api.github.com/users/simpleenergy/received_events","type":"Organization","site_admin":false},"repo":{"id":27644031,"name":"env-excavator","full_name":"simpleenergy/env-excavator","owner":{"login":"simpleenergy","id":522113,"avatar_url":"https://avatars.githubusercontent.com/u/522113?v=3","gravatar_id":"","url":"https://api.github.com/users/simpleenergy","html_url":"https://github.com/simpleenergy","followers_url":"https://api.github.com/users/simpleenergy/followers","following_url":"https://api.github.com/users/simpleenergy/following{/other_user}","gists_url":"https://api.github.com/users/simpleenergy/gists{/gist_id}","starred_url":"https://api.github.com/users/simpleenergy/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/simpleenergy/subscriptions","organizations_url":"https://api.github.com/users/simpleenergy/orgs","repos_url":"https://api.github.com/users/simpleenergy/repos","events_url":"https://api.github.com/users/simpleenergy/events{/privacy}","received_events_url":"https://api.github.com/users/simpleenergy/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/simpleenergy/env-excavator","description":"","fork":false,"url":"https://api.github.com/repos/simpleenergy/env-excavator","forks_url":"https://api.github.com/repos/simpleenergy/env-excavator/forks","keys_url":"https://api.github.com/repos/simpleenergy/env-excavator/keys{/key_id}","collaborators_url":"https://api.github.com/repos/simpleenergy/env-excavator/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/simpleenergy/env-excavator/teams","hooks_url":"https://api.github.com/repos/simpleenergy/env-excavator/hooks","issue_events_url":"https://api.github.com/repos/simpleenergy/env-excavator/issues/events{/number}","events_url":"https://api.github.com/repos/simpleenergy/env-excavator/events","assignees_url":"https://api.github.com/repos/simpleenergy/env-excavator/assignees{/user}","branches_url":"https://api.github.com/repos/simpleenergy/env-excavator/branches{/branch}","tags_url":"https://api.github.com/repos/simpleenergy/env-excavator/tags","blobs_url":"https://api.github.com/repos/simpleenergy/env-excavator/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/simpleenergy/env-excavator/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/simpleenergy/env-excavator/git/refs{/sha}","trees_url":"https://api.github.com/repos/simpleenergy/env-excavator/git/trees{/sha}","statuses_url":"https://api.github.com/repos/simpleenergy/env-excavator/statuses/{sha}","languages_url":"https://api.github.com/repos/simpleenergy/env-excavator/languages","stargazers_url":"https://api.github.com/repos/simpleenergy/env-excavator/stargazers","contributors_url":"https://api.github.com/repos/simpleenergy/env-excavator/contributors","subscribers_url":"https://api.github.com/repos/simpleenergy/env-excavator/subscribers","subscription_url":"https://api.github.com/repos/simpleenergy/env-excavator/subscription","commits_url":"https://api.github.com/repos/simpleenergy/env-excavator/commits{/sha}","git_commits_url":"https://api.github.com/repos/simpleenergy/env-excavator/git/commits{/sha}","comments_url":"https://api.github.com/repos/simpleenergy/env-excavator/comments{/number}","issue_comment_url":"https://api.github.com/repos/simpleenergy/env-excavator/issues/comments{/number}","contents_url":"https://api.github.com/repos/simpleenergy/env-excavator/contents/{+path}","compare_url":"https://api.github.com/repos/simpleenergy/env-excavator/compare/{base}...{head}","merges_url":"https://api.github.com/repos/simpleenergy/env-excavator/merges","archive_url":"https://api.github.com/repos/simpleenergy/env-excavator/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/simpleenergy/env-excavator/downloads","issues_url":"https://api.github.com/repos/simpleenergy/env-excavator/issues{/number}","pulls_url":"https://api.github.com/repos/simpleenergy/env-excavator/pulls{/number}","milestones_url":"https://api.github.com/repos/simpleenergy/env-excavator/milestones{/number}","notifications_url":"https://api.github.com/repos/simpleenergy/env-excavator/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/simpleenergy/env-excavator/labels{/name}","releases_url":"https://api.github.com/repos/simpleenergy/env-excavator/releases{/id}","created_at":"2014-12-06T18:16:34Z","updated_at":"2015-03-25T19:24:07Z","pushed_at":"2016-01-15T16:49:52Z","git_url":"git://github.com/simpleenergy/env-excavator.git","ssh_url":"git@github.com:simpleenergy/env-excavator.git","clone_url":"https://github.com/simpleenergy/env-excavator.git","svn_url":"https://github.com/simpleenergy/env-excavator","homepage":null,"size":307,"stargazers_count":3,"watchers_count":3,"language":"Python","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":3,"mirror_url":null,"open_issues_count":3,"forks":3,"open_issues":3,"watchers":3,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/simpleenergy/env-excavator/pulls/6"},"html":{"href":"https://github.com/simpleenergy/env-excavator/pull/6"},"issue":{"href":"https://api.github.com/repos/simpleenergy/env-excavator/issues/6"},"comments":{"href":"https://api.github.com/repos/simpleenergy/env-excavator/issues/6/comments"},"review_comments":{"href":"https://api.github.com/repos/simpleenergy/env-excavator/pulls/6/comments"},"review_comment":{"href":"https://api.github.com/repos/simpleenergy/env-excavator/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/simpleenergy/env-excavator/pulls/6/commits"},"statuses":{"href":"https://api.github.com/repos/simpleenergy/env-excavator/statuses/848120125c6cd74698718b19954fe57461e1def5"}}}}