It would be very helpful to have a setting in the Fastly module to disable the API purges during normal Drupal tasks for local environments. We've found that having Fastly enabled in a local environment significantly slows down the normal browsing / drush commands on a local site.

With a setting, it could be overridden in a settings.php file per-environment.

Issue fork fastly-3099119

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

Anonymous’s picture

vilepickle created an issue. See original summary.

Anonymous’s picture

Note: aware that config_split exists, but we don't have config_split and don't really desire to add that overhead into our site at this time. It'd be nice to have this option.

duaelfr’s picture

I agree with this feature request. Even while using Config Split, it's considered as a best practice to use the default config for production and splits for other environments. The thing is that Config Split does not allow to disable a module in a split, only to add some more. This setting would be very useful.

duaelfr’s picture

Status: Active » Needs review
StatusFileSize
new3.44 KB

Here we go!

Anonymous’s picture

Status: Needs review » Needs work

Thanks for the patch. Looks like the following is missing from the settings form submit handler:

->set('purge', $form_state->getValue('purge'))
Anonymous’s picture

Also, I think the checkbox field might only need to look like this instead of having the conditional. The conditional never shows "0" when retrieving a 0 purge value and instead shows checked:

    $form['purge']['purge'] = [
      '#type' => 'checkbox',
      '#title' => $this->t('Activate purging'),
      '#default_value' => $config->get('purge'),
    ];
duaelfr’s picture

Status: Needs work » Needs review
StatusFileSize
new1.1 KB
new4.02 KB

@vilepickle OMG you're right. Thank you for your review! I really did this on a rush ^^
Here is the new patch.

xsdx’s picture

Not sure if we need this because you can actually disable and enable modules on various environments (production, staging, development,...) with config split and 'blacklist'. You can check whole tutorial how to do it here: https://support.acquia.com/hc/en-us/articles/360020124953-Disable-and-En...

duaelfr’s picture

It seems that most people use config splits only for dev/testing environments (it's one less module on production!) and there is no way to disable modules using splits, only enable them or have a specific configuration. That's why we need this setting!

PS: I rerolled the patch on the last dev version

kay_v’s picture

looks like there's overlap with this issue, for what it's worth: #3176963: Fastly makes purge requests even if API key is not set

luksak’s picture

@kay_v I don't agree that #3176963: Fastly makes purge requests even if API key is not set overlaps with this issue. I think that adding a way to easily disable fastly purging makes sense. This is what most modules with similar functionality like this use for a good reason I guess.

#3176963: Fastly makes purge requests even if API key is not set makes sure there are no errors if the seetings were not set properly. This is about disabling fastly purging altogether.

luksak’s picture

Status: Needs review » Reviewed & tested by the community

#9 Works like a charm!

duaelfr’s picture

StatusFileSize
new3.93 KB

Rerolled patch on last dev version.

luksak’s picture

Could we please get this committed? This is a major issue for me, since it breaks my CI...

luksak’s picture

Priority: Normal » Major
ksenzee’s picture

Status: Reviewed & tested by the community » Needs review
StatusFileSize
new5.2 KB
new1.16 KB

Maybe I'm misunderstanding the use case here, but I don't think this patch goes far enough. It also needs this check in the FastlyPurger's invalidateItems() method, for those who are using the purge module. Attaching an updated patch.

vuil’s picture

Status: Needs review » Reviewed & tested by the community

#16 works. I set the issue to RTBC.
There is no issues found.

thatguy’s picture

StatusFileSize
new5.22 KB

Re-rolled #16 for 4.x

vvuksan-fastly made their first commit to this issue’s fork.

simonbaese’s picture

Status: Reviewed & tested by the community » Needs work

Used this patch for a while now. It seems like the configuration added in the test does not properly deactivate the diagnostics for the purger plugin. It may has to do with the runtime measurement injection, see comment for the hasRuntimeMeasurement() method.

When invalidating a lot of tags on a development environment with the purge option disabled, one may run into errors from the QueueSizeDiagnosticCheck. For example:

Your queue exceeded 100 000 items! This volume is extremely high ...

I do not know the plugin system for the purgers well, yet. Maybe we could make the return value of the hasRuntimeMeasurement() in the FastlyPurger plugin depend on the configuration.

vvuksan-fastly’s picture

Status: Needs work » Closed (won't fix)

Hey folks. In the light of that config_split provides support for splitting the configs and managing all aspects of the configuration I will close this issue.

https://www.drupal.org/project/config_split

ksenzee’s picture

@vvuksan-fastly Could I possibly ask you to reconsider? Most modules have an off switch of some kind that can be controlled via a settings.php file, and config_split isn't that commonly installed. I just checked some statistics, and the most common contrib modules have 300k-600k reported installs, while config_split only has 60k. Our site doesn't use config_split, so we'll now be in the position of installing it just to support this one module (which means a fairly significant adjustment to how all our developers work) or maintaining and running this patch forever. Honestly we'll probably choose the patch route because it's less disruptive.

jwilson3’s picture

Raising hand for reconsideration. We'll also be running this patch forever. As stated elsewhere, config_split is not a valid approach because it provides no mechanism to leave the module enabled on local but having the features disabled just through configuration. Puzzlingly enough, for the config_split approach to work optimally, it would actually also benefit from the patch in this issue.

Furthermore, I don't want to install another module and manage the overhead of having multiple core.extensions.yml files in multiple different environment-specific config_split folders in order to disable this one module on local. Once you go down this path, it becomes a "gotcha" to ensure that new modules enabled on all environments are added to all core.extensions.yml files, and that these stay in sync when a new module is actually intentionally enabled on both environments.

All other Drupal contrib modules we use have ways to disable functionality via feature flags on local via settings.local.php

bvoynick’s picture

I would also like to see this reopened. Config Split cannot address the issue that uninstalling Fastly & related Purge modules, in an environment lacks the relevant credentials secrets, produces a ton of noisy errors related to inability to purge.

khiminrm’s picture

+1 to have this feature without installing config_split

azinck’s picture

Here's another vote for reconsidering this decision. I agree with the points made above. Config_split is a very non-optimal way to solve this problem.

anna d’s picture

StatusFileSize
new5.3 KB

Re-rolled #16 for 4.3

thomwilhelm’s picture

Also +1 for adding this. Much easier to have code like this rather than having to set up a config split.

// Disable Fastly purging by default.
$config['fastly.settings']['purge'] = FALSE;
if (is_production()) {
    // Enable purging on production.
    $config['fastly.settings']['purge'] = TRUE;
}
kyberman’s picture

Hi @vvuksan-fastly,

I also don't think the decision was correct because 1) config_split is not always the way to go, and 2) best practice for modules similar to fastly is the approach suggested above. Could you please consider it again?

Thank you.

bserem’s picture

Status: Closed (won't fix) » Needs review

@vvuksan-fastly please forgive me for re-opening this, but I understand closed issues won't get much attention.

Fastly is definitely a niche module and doesn't apply to all projects, but I do agree with all of the above people. Maintaining a split has always proven to be cumbersome and it is something we prefer to avoid for simple things.

AdvAgg, another performance related module supports disabling the module on settings.php level (source: https://git.drupalcode.org/project/advagg/-/blob/5.0.x/src/Form/Settings....

In reality, Fastly is something you will only need on a live environment. It would be great to be able to do just that on module level, and not require an overly complicated extra module.

Again, forgive me for opening this again, feel free to close it again and I believe we'll all understand and stop requesting this. I certainly hope you reconsider though.

Thanks

apotek’s picture

Like many others here, I don't think forcing users to turn off purging on cache rebuilds only by means of disabling the entire module (and thus all its services) is the right approach at all. Almost all other modules that connect to an external service have a means of turning production functionality off if either a setting is set, or an environment variable is unconfigured. One shouldn't have to install config split and then set up different configurations just to ensure that purges aren't triggered in testing and dev when they only lead to errors.

Nor should we be forcing developers to have to set up a Fastly service just for QA or just for Dev. That's a lot of extra cost and very very few developers get to make purchasing decisions anyway.

Here's another use case. I have written a drush command that depends on the presence of some of the services defined in the Fastly module. I need my custom drush command available in dev for testing. Which means, that the Fastly module needs to be enabled in dev for my testing to work. If I disable the Fastly module, the code I have that depends on the services defined in the Fastly module fails. In fact, I can't even deploy the site due to missing service dependencies if the Fastly module is disabled.

Having a Fastly purge ON/OFF switch makes sense, makes everybody's life easier, and is necessary in cases where other code depends on the Fastly module. It is not hard to implement. Patches have been contributed. The resistance to this feature is really strange.

jwilson3’s picture

Said in another way, the concept of feature flags in Drupal are implemented as $config['module.settings']['enabled'] = FALSE;. Those feature flags can then also optionally be exposed in the admin UI, to facilitate quick debugging. Ultimately though, adding a module settings configuration is the only realistic approach to solving this.

The patch in #29 does exactly this. If I had to criticize it at all, it would be to rename the setting 'purge' to 'enabled', to match the property name introduced in the FastlyPurger class.

bedlam’s picture

Another voice to say that config split is a very poor solution for this problem, in spite of its general usefulness.

But config_split or patch, I think this issue is the result of a related issue in the module having to do with its use of Drupal's state system, and how a certain state value can remain unexpectedly true in non-Prod environments.

apotek’s picture

As this thread has been open for seven years and maintainers seem skeptical of the proposed solution, I wanted to highlight the way I have been working around this limitation in my site builds. I have found this approach preferable to patching as I remain in control of my implementation. Hope this helps someone.

Like others have observed, having different modules enabled in production versus testing or development leads to, essentially, untested production deployments as the code has not been vetted for circular service dependencies, etc. So using module state to turn the API calls on and off is not advisable.

But you can work around it in a way that does not require a patch.

1. Decorate Fastly's `state.api` service with a service of your own.
2. Implement your FastlyStateApiDecorator service in a way that overrides what Fastly's state service returns and sets by taking into account your own custom configuration setting:

  public function getPurgeCredentialsState(): mixed {
    $state = $this->fastlyState->getPurgeCredentialsState();
    if ($state) {
      $fastly_service_active = $this->configFactory->get('yourmodule.settings')->get('fastly_service_active') ?? FALSE;
      return ($state && $fastly_service_active);
    }
    return $state;
  }

  public function setPurgeCredentialsState($state = FALSE) {
    if ($state) {
      $fastly_service_active = $this->configFactory->get('yourmodule.settings')->get('fastly_service_active') ?? FALSE;
      $state = ($fastly_service_active && $state);
    }
    $this->fastlyState->setPurgeCredentialsState($state);
  }

You will of course also have to add this config setting to your custom module's config schema.

This way your custom setting overrides Fastly's state.api service `getPurgeCredentialsState()` and will effectively "turn off" API calls if you have not set your custom setting to true. All without modifying the Fastly module at all.