search

openaddresses / machine

Scripts for running OpenAddresses on a complete data set and publishing the results.
http://results.openaddresses.io/
ISC License
97 stars 36 forks source link

accounting for the chain function in EsriRestDownloadTask.field_names_to_request #656

Closed albarrentine closed 4 years ago

albarrentine commented 7 years ago

In attempting to test the new chain function, I realized that one other change to machine is required for it all to work. The reason for the failed build in https://github.com/openaddresses/openaddresses/pull/3203 is in EsriRestDownloadTask.field_names_to_request. This method assumes each conform function has a property named either "fields" or "field", which does not apply to the compound chain function.

This change collects fields from the chain's constituent functions.

migurski commented 7 years ago

Looks good! Can we get a unit test added for this change?

albarrentine commented 7 years ago

added a test though unclear why no build is running.

migurski commented 7 years ago

Whoa, weird.

migurski commented 7 years ago

I'm unsure why Circle isn't building. I've turned it off an on again, and I'll need to try jiggling it with a new commit to see if it works.

migurski commented 7 years ago

Well this is frustrating. Sorry the tests have somehow stopped — I'll keep digging when I have a free moment, to figure out what’s happening.

albarrentine commented 7 years ago

In any case I've tested the change locally and it looks fine.

iandees commented 4 years ago

Closing in favor of #764.