Skip to content
This repository was archived by the owner on Jan 23, 2019. It is now read-only.

Fixed the API call to Yahoo#79

Closed
calebjacob wants to merge 2 commits intomonkeecreate:masterfrom
calebjacob:master
Closed

Fixed the API call to Yahoo#79
calebjacob wants to merge 2 commits intomonkeecreate:masterfrom
calebjacob:master

Conversation

@calebjacob
Copy link
Copy Markdown

Hey man,

I noticed that the call to Yahoo's API was failing, so I simply reformatted the URL structure a little. I removed some unneeded parameters and made sure that there were no decoding/encoding issues. All the existing functionality works like it should.

Feel free to merge in the fix and let me know if you have any questions. Thanks! 👍

@defvayne23
Copy link
Copy Markdown

I'm curious about the original issue. I see you updated the index.html to set the location to Denver. (Go Avs!) This voids the woeid since it checks if location exists first. Was the woeid used bad or maybe the location was included with a space or something?

@defvayne23
Copy link
Copy Markdown

Also you're including unit into the query twice.

@fleeting
Copy link
Copy Markdown
Member

I'm currently in the middle of a big rewrite of the code base and cleaning up a lot of stuff. I'm also not having any issue with any of the demos.

@fleeting fleeting closed this Apr 18, 2014
@calebjacob
Copy link
Copy Markdown
Author

The demo doesn't work: http://simpleweatherjs.com/

The API call that you're using to fetch the data isn't formatted correctly: http://query.yahooapis.com/v1/public/yql?format=json&rnd=20143511&diagnostics=true&callback=jQuery111009611508797388524_1397843378330&diagnostics=true&env=store%253A%252F%252Fdatatables.org%252Falltableswithkeys&q=select%20*%20from%20weather.forecast%20where%20woeid%20in%20(select%20woeid%20from%20geo.placefinder%20where%20text=%22Austin,%20TX%22%20and%20gflags=%22R%22)%20and%20u=%22f%22&_=1397843378331

You're right, the URL does have unit in their twice. Anyways, your plugin stopped working for a friend of mine and I did a quick patch. I'd be really surprised if the demo is working for you.

Anyways, just wanted to give you a heads up! Thanks.

@defvayne23
Copy link
Copy Markdown

I don't have a problem viewing the demo. http://d.pr/i/PyZT

@fleeting
Copy link
Copy Markdown
Member

@iamceege Can I ask what browsers you are using as we aren't seeing any issue and I'm currently coding on the next version and getting back data? It looks like it works for your friend earlier https://twitter.com/shallanelprin/status/457205767292989440. Not sure if she is using your version or mine.

@calebjacob
Copy link
Copy Markdown
Author

@fleeting I'm using Chrome on OSX. That's really weird how you're not seeing the same issue - hmm.. My friend and I are both seeing the broken API call on the demo.

She's still using my version which is generating this URL for the Yahoo query (haha, you can see my mistake with "unit" being in there twice): https://query.yahooapis.com/v1/public/yql?q=select%20*%20from%20weather.forecast%20where%20woeid=12798301%20and%20u=%22f%22%20and%20u=%22f%22&format=json&diagnostics=true&callback=

Going to that endpoint in my browser works for me, whereas the endpoint that your version is generating doesn't work for us (I pasted the endpoint in my previous comment).

@calebjacob
Copy link
Copy Markdown
Author

Here's a screenshot of what the demo is giving me:

screen shot 2014-04-18 at 12 14 00 pm

The console is dumping a "Failed to load resource: net::ERR_CONNECTION_RESET" error.

@calebjacob
Copy link
Copy Markdown
Author

@fleeting AHHH, I just figured it out. The Yahoo API needs to operate over the HTTPS protocol. Since you don't have the protocol set in the URL setup, my browser is defaulting to HTTP and failing. I'd probably suggest hard coding the protocol into the URL string you're building.

Sorry for the noise! Thanks. :)

@defvayne23
Copy link
Copy Markdown

That's is extremely odd. Both HTTP and HTTPS are working for me. Could you try outside your network? Like on your phone over cellular.

@fleeting
Copy link
Copy Markdown
Member

Thanks for helping diagnose the issue! I was really curious why it wasn't working for you guys. It works fine in my Chrome OS X. I wonder if it's a browser setting or extension between us causing the issue because http and https both work for me.

I think I'll go ahead and change it to always use https on the API call anyways. I hope to have this rewrite done over the weekend so will include that in the v3.0 release.

@calebjacob
Copy link
Copy Markdown
Author

No problem!

@defvayne23 So it does appear to be a network issue on our side. I was able to access the API via HTTP and HTTPS on a different network. I can't speak to exactly what's causing the issue on our side...but that doesn't really matter for you guys anyways. First time I've run into anything like that. Weird!

@fleeting
Copy link
Copy Markdown
Member

Thanks for the info @iamceege. I'll go ahead and set the API call to always use https in the next release to avoid somebody else running into this issue.

fleeting added a commit that referenced this pull request May 18, 2014
…tps API call bug [#79]. Minimium jQuery required is now v1.2.0.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants