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

Remove geocoding completely! #59

Open
gkop opened this issue Aug 6, 2014 · 13 comments
Open

Remove geocoding completely! #59

gkop opened this issue Aug 6, 2014 · 13 comments

Comments

@gkop
Copy link
Member

gkop commented Aug 6, 2014

Currently we are geocoding venues in our test suite through google's API.

There are several ways to fix this.

We can stub geocoding in test everywhere.

Or maybe we don't need to geocode venues automatically.

gkop pushed a commit that referenced this issue Sep 10, 2014
Ameliorates #59

Also don't link IDL script in test env
@gkop
Copy link
Member Author

gkop commented Apr 3, 2015

I am thinking now we should remove geocoding completely. Yes, sometimes it's convenient - but it's not only slow but also inaccurate, and the inaccuracy causes confusion.

How about instead, we display a modal "Pick your school"?

@btaitelb
Copy link
Member

btaitelb commented Apr 7, 2015

Should we rethink how we're handling different schools, since both SF and Cville are having classes online? It seems like a lot of people (especially in other cities) may be missing out on good learning opportunities by being forced to choose a school. At the same time, the bulk of our students are still physically coming to classes and shouldn't have to deal with spam about classes from another part of the country/world unless they explicitly want them.

@gkop
Copy link
Member Author

gkop commented Apr 7, 2015

Yes, we should allow people to pick and choose which schools they want to subscribe to.

@gkop gkop changed the title Remove google geocoding dependency Remove geocoding completely! Apr 21, 2015
@donaldali
Copy link
Contributor

I think the geocoding is being inaccurate because the value we use with the near method is too high. The distance between SF and Cville is about 2800 miles, but we are passing 5000 miles into near to find a school for a user. As currently set up, the school assigned would be dependent on how schools are sorted (as opposed to which school is closer to a user) because both schools would be within range of anywhere in the continental US. I think a parameter of 1400 miles to the near method would make the geocoder work as expected.

If we do decide to take out geocoding completely, it seems to me that the drop down button in the header would suffice for users to choose their school. If the user is a guest, then we can store that school in a cookie (in addition to the session).

I think, for now, we should try the geocoding with the 1400 miles and if it continues to be unreliable, then there would be no need to keep it.

@gkop
Copy link
Member Author

gkop commented Apr 22, 2015

Sounds good to me. Thanks Donald!

@cdale77
Copy link
Contributor

cdale77 commented Jun 9, 2015

@donaldali @gabe can we close this?

@btaitelb
Copy link
Member

btaitelb commented Jun 9, 2015

I got a report from Nene that when she pulled up RS on her phone last week, it opened to the cville site. I don't know if this was due to cookies being set from a previous time but I worry there might still be an issue. Can anyone in SF reproduce?

On Jun 9, 2015, at 5:08 PM, Brad Johnson [email protected] wrote:

@donaldali can we close this?


Reply to this email directly or view it on GitHub.

@gkop
Copy link
Member Author

gkop commented Jun 9, 2015

I feel like it's working better in SF with Donald's patch. How about out on the east coast, in a clean browser session, does the app show you Cville?

@gkop
Copy link
Member Author

gkop commented Oct 8, 2015

We definitely at least should set a timeout for the geocoding < ~10 seconds, because unicorn times out at 15 seconds.

@btaitelb
Copy link
Member

btaitelb commented Oct 8, 2015

if google doesn't respond within 1 second for geocoding, it's probably an issue, so it'd be better to retry once or twice and then fail fast, rather than waiting for it to time out.

@gkop
Copy link
Member Author

gkop commented Oct 9, 2015

I'm not sure I understand. Right now we're not timing out the google call, google isn't failing it's just taking longer than 15s to respond. So I propose we do timeout the google call.

@btaitelb
Copy link
Member

btaitelb commented Oct 9, 2015

If the google call is really taking longer than 15s to respond, then
something's wrong. This should be a fast lookup, so I'd consider anything
longer than 500ms to be an issue that we should mitigate.

On Thu, Oct 8, 2015 at 5:05 PM, Gabe Kopley [email protected]
wrote:

I'm not sure I understand. Right now we're not timing out the google call,
google isn't failing it's just taking longer than 15s to respond. So I
propose we do timeout the google call.


Reply to this email directly or view it on GitHub
#59 (comment).

@gkop
Copy link
Member Author

gkop commented Oct 9, 2015

Sounds good.

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

No branches or pull requests

4 participants