briandowns/openweathermap

no data returned

Nesurion opened this issue · 15 comments

when running the cli example I'm not getting any data:
go run weather.go -l=EN -u=C -w=Bonn

Current weather for :
    Conditions:
    Now:         0 metric
    High:        0 metric
    Low:         0 metric

since 3 days ago this worked perfectly

Thank you very much for reporting this. Looks like OWM has finally implemented the requirement of an API key. Since they hadn't for a long time, I don't have that fully integrated. That's not to say that that's the issue but the call you're making above is getting this in return.

&{401 Unauthorized 401 HTTP/1.1 1 1 map[Server:[nginx] Date:[Mon, 12 Oct 2015 21:41:09 GMT] Content-Type:[text/plain] Content-Length:[81] Connection:[keep-alive]] 0xc8200c40c0 81 [] false map[] 0xc8200ae000 <nil>}

When making the same call from a browser, this is the returned message:

Invalid API key. Please see http://openweathermap.org/faq#error401 for more info.

So I'm really leaning towards API key being missing being the problem.

If this is the case and there's now a requirement for the an API key, what are everyone's thoughts on impmenenting this? My immediate thought is to just look for an ENV var. Having different options might be nice but a preferred method should be agreed upon as a standard.

Ok, just received word back from OWM that they have always needed an API key but they weren't enforcing it but NOW they are. I'll try to get a fix in place soon.

@briandowns thank you for the quick response. Providing the API key as an ENV var sounds good.

Providing the API key as an ENV var is "not" good.

I think it is better to have a call that sets API keys and the following requests could use it.

Or it could be per call argument.

Parsing it from ENV is evil.

The reason I was thinking as an ENV var is because it never has to exist in the code and never has to exist in a file for the code to read providing more protection for the key. This is how it's done for AWS, for OpenStack, and for a number of other packages that require this kind of data.

My goal with now having to require a key is for it to be a set it and forget it type of thing for the user since they're not going to want to fuss with their key all the time, nor do I want to have it getting in the way of the functionality of the lib. If it's in an ENV var, it can be set as part of the base URL call and not much in the package has to change keeping backwards compatibility.

If that's just poor form, that's fine, but let's come up with solutions to keep users code from breaking if possible and keeping keys safe and secure.

Ok. That sounds good.

You seem to have a structure called Config. Are you planning to use it for all request function calls?

Yes. This is where I planned on throwing the API key once parsed from the ENV. This is also where I planned on putting the necessary data for posting weather statistics from weather stations, etc.

Ok. I have made a temporary working version with API key. Can you let me know how far you have come with your implementation? I can push a pull request, if you are in early stages.

Sure! Throw a PR up and we'll take a look.

Done. Take a look.

Also fixed test cases to report FAIL when the fetch request returns error.

@Nesurion, we should have this situated today. Apologies for the delay.

@briandowns thanks, no worries.

Resolved in #30