duckduckgo / zeroclickinfo-spice

DuckDuckGo Instant Answers based on JavaScript (JSON) APIs
https://duckduckhack.com/
Other
548 stars 942 forks source link

Weather Trigger Discrepancy #362

Closed loganom closed 10 years ago

loganom commented 10 years ago

Now that I know how I can get an abbrev. weather forecast for any zip code right in DDG, i'm ecstatic, though I thought the intention was for the following two queries to produce the same results. I may look at the triggers for Forecast.pm, this shouldn't be a terrible thing to fix.

weather+63501 gives me the weather for 63501 forecast+63501 gives me the weather for my current location (not 63501).

https://duckduckgo.com/?q=weather+63501 https://duckduckgo.com/?q=forecast+63501

moollaza commented 10 years ago

@loganom yes this definitely looks like a bug, thanks for bringing it to our attention!

If you'd like to try and fix it, feel free to go ahead and make a pull request. If not, someone from the community or DDG Team will get around to it.

loganom commented 10 years ago

I'll work on submitting a pull request this afternoon - should be quite simple. On Jan 2, 2014 11:05 AM, "Zaahir Moolla" notifications@github.com wrote:

@loganom https://github.com/loganom yes this definitely looks like a bug, thanks for bringing it to our attention!

If you'd like to try and fix it, feel free to go ahead and make a pull request. If not, someone from the community or DDG Team will get around to it.

— Reply to this email directly or view it on GitHubhttps://github.com/duckduckgo/zeroclickinfo-spice/issues/362#issuecomment-31466699 .

loganom commented 10 years ago

@moollaza I'm sure this will be the only time I'll have to ask this, but how can I work with a valid apikey while running the duckpan server?

current error: "http://forecast.io/ddg?apikey=&q=19460&callback=ddg_spice_forecast" 403 Forbidden

moollaza commented 10 years ago

@loganom sorry, I forgot this Spice requires an API Key. Unfortunately I can't share our key with you, but normally you would be able to get your own and use the duckpan env command (see duckpan help) to add an API key to DuckPAN's Environment.

However, this is a unique case because Forecast.io created a special API for us and they don't give out keys to devs, so it seems you've hit a roadblock. We're going to have a talk about how we want to handle these situations in the future because it hasn't really come up before.

For now if you want to go ahead and submit your untested PR that would be totally fine given the situation. How does that sounds?

loganom commented 10 years ago

That was my suspicion. Earlier I was able to use duckpan env successfully, but as you mentioned Forecast.io has created an API for DDG and the responses vary so my requests were being posted (under the new API)-- but I couldn't handle the responses with the current pm.

Could you expose the documentation for your API as reference?

I'll see what I can churn up.

moollaza commented 10 years ago

@loganom we don't really have any documentation for the API they provided -- its really just that endpoint that you saw in the DuckPAN request.

We simply pass along a string representing a location, and their backend does the work of determining what that location is and then sends us a JSON response which you actually can see on the live site (if you look at the source) when the Spice triggers:

https://duckduckgo.com/js/spice/forecast/90210

You can replace 90210 with anything to see how their API responds. The JS portion of the Forecast spice uses this data to populate the template and create the display.

Does that help?

loganom commented 10 years ago

@moollaza -- That's perfect. I can work with that.

jagtalon commented 10 years ago

@loganom has fixed this.

jagtalon commented 10 years ago

@loganom This was released last Friday. Check it out!

loganom commented 10 years ago

I was glad to see it, @jagtalon :)