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

Use quadrant-based method as a last resort after OID enumeration is tried #37

Merged
merged 6 commits into from
Mar 1, 2017

Conversation

albarrentine
Copy link
Contributor

Also from #34, but putting this up for discussion in case there's something I missed. The way this was set up previously, if the source didn't support returnCountOnly, pyesridump would use the quadrant scraping method, which makes several geospatial queries, when there still might be more efficient methods left to be tried e.g. min-max or OID enumeration (now faster with the introduction of #33).

This PR changes the order of operations to the following:

  1. If the source supports returnCountOnly, use offset-based method
  2. If the source supports statistics queries, use the minmax method
  3. If the source supports returnIdsOnly, fetch them all and use OID enumeration method
  4. If none of the above are supported, fall back to the quadrant method

Based on the comments in dumper.py I think this is how it was intended to work.

@albarrentine
Copy link
Contributor Author

Although based on the test cases maybe that's not how it's supposed to work.

@iandees
Copy link
Member

iandees commented Mar 1, 2017

I think you've got the order right, but the trigger for offset/pagination-based method is through a couple capabilities checks in the layer metadata here.

@albarrentine
Copy link
Contributor Author

Right. The only problem is if the server doesn't support returnCountOnly, those checks never get executed because of this error handling block: https://github.com/openaddresses/pyesridump/blob/master/esridump/dumper.py#L244-L264

…sOnly as well before making the geo queries)
@iandees
Copy link
Member

iandees commented Mar 1, 2017

I think I do that because I need to know how many elements there are to create the paginated requests. If you can think of a way to do it without that knowledge I'm happy to move that around.

@albarrentine
Copy link
Contributor Author

That ought to do it.

@iandees iandees merged commit 2975a43 into openaddresses:master Mar 1, 2017
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