prevent geocoding during migration

Is it an option to turn off the geocoding widget during your migration, then turn it back on afterwards for followup data? If you do that, you'll probably need to translate your lat/lon data into something geofield can understand manually, but that's not that big of a deal. See https://drupal.org/node/1648976#comment-6297196 for details.

I had exactly the same issue but in my case, I added two functions in geocoder.module: geocoder_status_get() and geocoder_status_set() so I was able to disable the module before importation with hook_feeds_before_import() and to enable it again at the end with hook_feeds_after_import().

This patch was created from the stable 7.x-1.2 (not the dev branch).

I agree with you about variable_get() issue but in my case it was a one shot migration on a staging server so it was not a problem. Let's find out another solution...

Since migrate #2065295: Disabling hook invocations for specific modules has been committed to the migrate dev version you can avoid the geocoding during a migration by disabling the hook_field_attach_presave()

See documentation: Disable hooks during migration
https://www.drupal.org/node/2136601

Example:

/**
 * Implements hook_migrate_api().
 */
function my_migration_migrate_api() {

  // Disable geocoding during migration as we would hit the Google API limit.
  $disable_hooks = array(
    'field_attach_presave' => array(
      'geocoder',
    ),
  );

  $api = array(
    'api' => 2,
    'groups' => array(
      'my_group' => array(
        'title' => t('My Group'),
        'disable_hooks' => $disable_hooks,
      ),
    ),
    'migrations' => array(
      'MyContentMigration' => array(
        'class_name' => 'MyContentMigrationMigration',
        'group_name' => 'my_group',
      ),
    ),
  );

  return $api;
}

I think we therefore can close this.

This is yet another patch to disable geocoder globally using variables. Unfortunately I wanted to disable geocoder only during migration run with update option, so couldn't use the solution provided by migrate's disabled_hooks.
Note: patch is not for latest dev but for c1a79dc

@lex0r, thanks for the patch! That option works great for us.

Our usecase is that during upcoming site migrations we need to import over 25.000 nodes with geolocation data that has already been geocoded before. If we use node_export to export the content from the old site over to the new site it would mean that the geocoder API would make a call to Google for every single node_save, which would result in 25k calls. This while Google only allows 2.5k per day for the free service.

These calls add a lot of unnecessary delay during the import, while also causing our website not to be able to make further calls that day.

I have tested the patch against the latest dev, and can confirm it's working properly.

(I'm uploading a new version of the patch with a small newline change for better readability).

---

Can we get this feature committed please? While it may not be in high demand, it can sure be a lifesaver in certain situations and doesn't add that much complexity IMO.

+1 looks good and very useful!

Just wanted to chime in and say that the patch in #9 worked well. I wish I would have found this before I hit my limits today.
I assumed that if I was providing the data to the field during a feeds import that it wouldn't make an API call.

Yet we have the disable hooks as described at https://www.drupal.org/node/2023505#comment-9053695 it is really cool to have both options.

Patch #9 mostly looks okay. However, a few issues:

A) '#description' => t('Uncheck if you want to disable requesting external services during entity updates in case they have and geocoder fields'),

That language is clumsy and a bit confusing.
There's also a typo: s/and geocoder fields/any geocoder fields/
Also, form #description text should be a complete sentence with a period at the end.

Perhaps something like this would be clearer:

"Uncheck to globally disable geocoding when updating entities (for example, to prevent queries to external services during content migration)."

B) I wonder if the setting name "geocoder_enabled" is confusing. From a DX standpoint, in Drupal the "enabled/disabled" distinction is generally used for if the module is enabled/disabled. This is more like "active" or something since the module would still be "enabled" in the usual Drupal sense. So maybe "geocoder_active" would be a better setting name.

C) Whatever it's called, we need to purge the new config setting via geocoder_uninstall() whenever geocoder is uninstalled.

Also, I marked #2825645: Create a kill switch to disable field based geocoding duplicate with this, although there's a simple patch there for the 8.x-2.x branch. It'd be good to agree on the config setting name and make it consistent for both branches.

Thanks,
-Derek

I came across this issue as I am writing a script to edit and save many thousands of nodes. I need this killswitch in order to temporarily disable calls to Google Maps while the script is running.

This patch uses the variable name geocoder_field_killswitch for consistency with #2825645: Create a kill switch to disable field based geocoding.

I prefer the term killswitch, because it emphasises that we only use it to switch off the functionality temporarily (for exceptional or safety reasons).

Also, I think it is best to have the variable not set to anything by default. So by default it is unchecked. Checking it should be the exception.

I amended the #description text in line with the suggestion in #9.

Also added the variable_del('geocoder_field_killswitch'); in function geocoder_uninstall().

Forgot to close my parenthesis and add a full stop in the #description text.

Forgot to rename the form element. Sorry for the noise.

Patch in #18 applies cleanly and Turns off geocoding. Turn off might be better than Disable as per comment in #14.

By the way, I wrongly assumed that Force re-geocoding's outcome would do just that (not geocode) when left unchecked i.e. do not geocode if the field was previously geocoded or geocoded values exist.

• What's clear: when checked, geocoding occurs whether the field was previously geocoded or not.
• What's not clear: what exactly happens when this option is unchecked?

Anyone?

Hello,

What's the status of this issue and the latest patch ?

I'd like to get a last feedback before testing/committing.

Thanks!

I've created a patch for 8.x - 2.x (on 2.0.0-beta3 base).

When applying, ensure to rebuild the cache afterwards. Adds a form checkbox to the geocoder settings page to disable it globally.

@Pol after testing this one you may include this one in your 8.x dev.

Sorry, wrong patching paths ... Here's the corrected version.

Hi,

If the schema is modified, then, there must be an upgrade path I think, or else, people will have issue when upgrading.

Updated the patch from #22 to work for 8.x-2.x, including a new geocoder.install file with an update function to update the configuration. This patch also works for 8.x-2.0-beta6.

Updating issue status.

Just updated to 8.x-2.0-beta7 and the last patch still seems to apply cleanly.

Patch #24 does not apply to 8.x-2.1. Since all the comments in the past 9 months have been about the 8.x version, I'm updating the version in the metadata for the issue.

Upon further investigation, the reason the patch #24 did not apply to 8.x-2.1 is that it's already been implemented in the release. Thank you!!

re: #28: huh? There's no mention of this issue in the commits for geocoder. I just checked out 8.x-2.1 and 'kill' never appears anywhere in the entire directory tree. I think you're confused, your site build is automatically applying the patch, or something. Granted, the git log for this module is something of a mess, with all kinds of commits that aren't attributed to any issues, useless commit messages like "tmp implementation", and so on. But, if this feature was actually committed, grep should be able to find the string "kill" or "prevent", neither of which are in the d.o git repo for this project.

Back to 'needs review'...

Cheers,
-Derek

Right. This patch wasn't yet included in any commit of the module, as this issue was still in 'needs working/needs review' states.

@BenStallings ... in the module it is already implemented to not Re-Geocode a source field that didn't change upon an entity update ... but not this specific feature.

Thanks for the nice work done here.
Here is a rerolled patch that solves this issue and applies cleanly on the actual Geocoder dev 2.x branch, with the following amends and enhancements:
- changes the new geocoder settings from (IMO too much "naif") 'geocoder_field_killswitch' into the more understandable and traditional 'geocoder_presave_disabled' ...
- adds some warning to the user (and close the details container) in the DefaultField->getSettingsForm(...) method in case of 'geocoder_presave_disabled'

Finally going to commit this into dev.

PS: For those already applied the geocoder_update_8201, please reset the geocoder system.schema to 8200, as that update should be re-run and will act on the geocoder.settings.geocoder_presave_disabled configuration schema.

PS: thanks @dww ... it is so nice to have appreciations on the hard and valuable work done (as volunteering) on this module,
that in more than one year has been brought from alpha draft state to life and advanced stability.
Although we try our best (in our free time) it is always nice to receive kind advices that aims to improve and optimize our skill and work.
Will care even more on commits messages from now on ... ;-)

@itamair: You're welcome. ;) Seriously, sorry that came across poorly. I totally see how it would, and my apologies for that. I mean no offense, and believe me I'm deeply sympathetic to the frustration of doing volunteer labor around here and getting not much but complaints about it. A quick peek at my profile will show you about that.

That said, I would encourage you to spend a little more time and care with your commit messages. :) That's incredibly valuable information, and absolutely necessary for people trying to debug problems, understand what's happening, etc. If you spend 20 minutes working on a commit, please spend 20 additional seconds on a better message so that everyone else trying to contribute can understand what's happening. The commits are very public info. You wouldn't just call all the variables in a method $x, $y, $z and $foo. You'd let them be more self-documenting by giving them meaningful names. That helps avoid bugs. Same goes for commit messages. Speaking from experience, I'm *sure* you're going to wonder, a few years from now, what you did in some random commit and why. You're not going to remember what "tmp implementation" means, and you're going to waste more time trying to re-figure it out. Please do yourself (and everyone who might try to help you maintain this module or fix bugs) a favor and write meaningful commit messages.

Thanks!
-Derek

Yes suuure @dww I totally agree with you, and will make treasure of all your advices. They are all so valuables ...

For what concerns the “tmp implementation” commit messages ... and other similar, well (as you can realize) they simply weren’t supposed to be committed ... as coming from the local working branches.
But I merged and also merged all those messages, without splashing, and rebasing ... etc. etc.
We where also remote co-maintaining and pull-requests reviewing.
Put in count also some lack of skills and experience on these kind of advanced worldwide public Git workflows.
Good to be notified on all these, and making all this so constructive.
Thank you sincerely for making me a better commiter for the future. 😉

This patch is for 7.x-1.4, updated lines from patch 18.

Sorry the patch in comment #38 is having wrong comment number in patch name. Below can be used instead.
Thanks

File paths changed. updated patch for comment #38 which is updated version of #18. This patch is for 7.x-1.4 version of the module.