Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Topic/eco stuff #1068

Merged
merged 3 commits into from
Jan 31, 2014
Merged

Topic/eco stuff #1068

merged 3 commits into from
Jan 31, 2014

Conversation

ahinz
Copy link
Contributor

@ahinz ahinz commented Jan 30, 2014

No description provided.

try:
rslt = json.loads(urllib2.urlopen(url).read())
except urllib2.HTTPError as e:
if e.fp.read() == 'invalid otm code for region\n':
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Testing for a specific error message seems fragile. But the alternative (returning a JSON response object indicating success or failure, with details) comes down to the same thing -- both sides must agree on an interface and both sides need to change if the interface changes. This is simpler, and time is short...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really want to spend some time properly refactoring the go tiler. There is an issue out there for using http error codes instead which I'd like to support: OpenTreeMap/otm-ecoservice#11

@RickMohr
Copy link
Contributor

+1

ahinz added 3 commits January 31, 2014 13:49
Fixes OpenTreeMap#1061

In the future we may want to support using a 404 but that appears
non-trivial with the go library we're using
ahinz added a commit that referenced this pull request Jan 31, 2014
@ahinz ahinz merged commit 36bb970 into OpenTreeMap:master Jan 31, 2014
@ahinz ahinz deleted the topic/eco-stuff branch January 31, 2014 18:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants