Needs review
Project:
Fastly
Version:
8.x-3.x-dev
Component:
Code
Priority:
Major
Category:
Feature request
Assigned:
Unassigned
Reporter:
Anonymous (not verified)
Created:
5 Dec 2019 at 19:42 UTC
Updated:
17 Aug 2026 at 01:20 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
Anonymous (not verified) commentedvilepickle created an issue. See original summary.
Comment #2
Anonymous (not verified) commentedNote: 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.
Comment #3
duaelfrI 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.
Comment #4
duaelfrHere we go!
Comment #5
Anonymous (not verified) commentedThanks for the patch. Looks like the following is missing from the settings form submit handler:
Comment #6
Anonymous (not verified) commentedAlso, 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:
Comment #7
duaelfr@vilepickle OMG you're right. Thank you for your review! I really did this on a rush ^^
Here is the new patch.
Comment #8
xsdx commentedNot 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...
Comment #9
duaelfrIt 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
Comment #10
kay_v commentedlooks like there's overlap with this issue, for what it's worth: #3176963: Fastly makes purge requests even if API key is not set
Comment #11
luksak@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.
Comment #12
luksak#9 Works like a charm!
Comment #13
duaelfrRerolled patch on last dev version.
Comment #14
luksakCould we please get this committed? This is a major issue for me, since it breaks my CI...
Comment #15
luksakComment #16
ksenzeeMaybe 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.
Comment #17
vuil#16 works. I set the issue to RTBC.
There is no issues found.
Comment #18
thatguy commentedRe-rolled #16 for 4.x
Comment #22
simonbaeseUsed 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:I do not know the plugin system for the purgers well, yet. Maybe we could make the return value of the
hasRuntimeMeasurement()in theFastlyPurgerplugin depend on the configuration.Comment #23
vvuksan-fastly commentedHey 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
Comment #24
ksenzee@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.
Comment #25
jwilson3Raising 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
Comment #26
bvoynickI 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.
Comment #27
khiminrm commented+1 to have this feature without installing config_split
Comment #28
azinck commentedHere'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.
Comment #29
anna d commentedRe-rolled #16 for 4.3
Comment #30
thomwilhelm commentedAlso +1 for adding this. Much easier to have code like this rather than having to set up a config split.
Comment #31
kybermanHi @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.
Comment #32
bserem commented@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
Comment #33
apotek commentedLike 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.
Comment #34
jwilson3Said 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.
Comment #35
bedlamAnother 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.
Comment #36
apotek commentedAs 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:
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.