So, in conclusion.
1. The hook that runs database updates is called `hook_update_N()` and lives in the .install(!) file.
2. The hook that runs updates right after the updates and before anything else is done is called `hook_post_deploy_NAME()` instead of (the more logical) `hook_post_update_NAME()`.
(No, developers, `hook_post_deploy_NAME()` does _not_ run after the deployment, even though it is named as though it would and even though it is located in a file called MODULE.post_update.php, and not in the .install file where its sibling `hook_update_N()` is.)
3. Instead of simply providing a way to run steps truly after a deployment has run as requested, the solution is.... a new requirement! You must install drush, and be able to use drush in your environment. Or, you have to do a separate step in the UI.
4. The name of the hook that the Drush project decided to use to run events _after_ a deployment is called :drumroll: .... `hook_deploy_NAME()` and lives in a MODULE.deploy.php file! I kid you not.
After 111 comments and many rejected patches, this is where we are.
Just wanted to say that this tutorial is excellent. Thorough, readable, and with a complete code example to iterate on. Thank you very much.
Just reporting in that we are seeing this frequent visitor in our logs too.
On one of our client sites, core/install.php takes about seven seconds to load. We just had a site outage this morning because someone was hammering that url until our client's php-fpm pool ran out.
The page returns cache headers that make the response uncacheable by your CDN so repeated requests do go to origin every time.
So while you are absolutely right that the page is not a security risk, it can easily be abused for DDOS purposes. Someone could bring your site down with a relatively low request rate of 1 or 2 requests per second (too low for your rate limiter or WAF to think anything is wrong).
So I absolutely recommend that, in a public environment, requests to this endpoint be blocked. Or remove that file. Some options:
1. When you build your deployment artifact, script the removal of that file.
2. Or use a composer install hook to remove that file after composer installs your dependencies.
The merge request provided does not actually implement the logging. Given the complexity (and enormity) of the request parameters, it is highly unlikely that whatever the maintainers would choose to log would not suit many.
The better option here, then, is to dispatch a Response event and allow users of this module to write an EventSubscriber that listens for the api response event and then log whatever data they wish, as they see fit.
A change request has been logged against the orange-dam-php library.
Thank you @fjgarlin and @jonathan1055 for digging in to the issue with me. I appreciate it.
As a module maintainer, I do appreciate the gitlab CI automations, but given the ad hoc and voluntary nature of module maintenance, it would be good if this tooling defaulted to advisory notices (as agreed in this thread) for anything the module has not yet claimed or delivered upon.
As a maintainer, I do want my phpstan and phpcs to fail for all versions for which I claim compatibility in my existing code. But I do not want them to fail for versions for which I have not begun the update effort and for which I am not claiming compatibility.
I also note that since Drupal 11 is out, next major would be Drupal 12. Or am I misunderstanding what next major signifies?
Thank you again for your contributions. I just wanted to offer some feedback in the issue itself as history. Thank you for #3471235 too.
Thank you for the great work! This will make module maintenance much clearer!
With update to migrate_source_queue, this is working in Drupal 10, and is non-breaking in Drupal 11.
Thank you for making the update @dieterholvoet! You have fixed the issue we were having with orange_dam module.
fjgarlin → credited apotek → .
I see the @dieterholvoet has made a new release (1.0.2) of migrate_source_queue!
I'll update and see how testing proceeds.
apotek → created an issue.
Would we be able to merge this? The merge request looks straightforward and we could bump the compatibility up to Drupal 11.
Hi fjgarlin,
Thank you for the response and clarification. It was helpful to note: "This job depends on the "composer (next major)" being successful".
This makes sense, but the thing is that the composer (next major) job is allowed to fail without failing. But this job marks a big red x failure when it fails. If this job depends on the success of the first (composer) job, then there is some dissonance here. It seems if the first job is allowed to fail, then this job should also be allowed to fail.
It looks like I will have to disable my CI. It was working fine before, but is now marking the merge requests as failing, despite having no discernible issues is the versions the module has demonstrated compatibility for.
Overall, it seems odd to fail a module for not having Drupal 11 compatibility when it doesn't even _claim_ to be compatible yet. If the module claimed version 11 compatibility and yet failed the composer (next major) job or failed the phpstan (next major), that would make sense. But in this case, these two jobs are introducing requirements that are not claimed by the module,which means these two jobs are _advisory_ and should not be failing merge requests.
The specific run is here: https://git.drupalcode.org/project/orange_dam/-/jobs/2803541
Unfortunately, this did not get ported to the 8x-1.7 release and there is no stable 2 release. So while this is closed, it isn't really fixed for any sites which require stable releases.
Is there a plan to make a 2.x release? As more and more people update to PHP 8.2/8.3 (Acquia is moving everyone onto those versions now), this issue will only get more widespread.
Gloria in excelsis mundi !!! Thank you to everyone for this epic improvement.
I reviewed and approved, though I did, like others, also feel that fixing the default scheme is a bit out of scope here.
Noting also that it's been a year since any maintainer closed or merged an issue on this project, so while this is good work here, we might be out of luck getting this into the project, so thank you to everyone who put up patches. I did write a long time ago asking about co-maintaining, but understandably did not hear back. Perhaps there is someone on this thread who would have the stature in the community to be able to make the same offer so we can address some of the issues that are beginning to stretch this module now with 10.3 out.
This was fixed while fixing issue 3439306 ✨ Provide public method to retrieve data only Fixed .
https://git.drupalcode.org/project/orange_dam/-/merge_requests/58
gpietrzak → credited apotek → .
gpietrzak → credited apotek → .
gpietrzak → credited apotek → .
gpietrzak → credited apotek → .
gpietrzak → credited apotek → .
gpietrzak → credited apotek → .
gpietrzak → credited apotek → .
Just reporting back, in case anyone else is experiencing this issue:
The status check fails in two odd cases.
Case 1: Where the exported config set and the installed config set are identical except for the order of some of the properties on the XML nodes. The value of the properties and the properties themselves are identical, just in a different order. So functionally, the XML is identical but the status report will still trigger a warning, which is unwarranted in this case.
Case 2: In my QA environment, I have configured the solr server to symlink /var/solr/data/mycore/conf to a mounted solr-conf directory. The contents of that mounted solr-conf directory consists exactly and exclusively of the config-set I exported from that very server before rebuilding it. So I downloaded the config-set from the QA server. I expanded the zip file into my solr-conf directory, rebuilt my QA solr server, and I still get the status report warning. If I shell into my QA solr server and compare the files in /var/solr/data/mycore/conf to the files I get when downloading the zip, they are identical.
I can only conclude that the comparison in this function is a little off and I advise people to ignore the warning if they have aleady made a good faith effort to resolve it with a new config.zip retrieval.
This was handled in https://www.drupal.org/project/orange_dam/issues/3441899 ✨ Make OrangeDamApi::getKeywords a pure get function Fixed
Already done. Though we could start planning version 3 :)
This patch seems to work as advertised. I don't know why this issue was taken from needs review to needs work without feedback. What is the patch lacking? Can that be made explicit? Putting this back to needs review.
I am also confused by this. If hook_install/uninstall are documented as being the way by which we can install custom entities or custom fields on entities (and remove them), but we are being told *not* to make such changes in these hooks during a config:import (which is the way most sites these days install and uninstall modules, not via explicity drush commands or the web gui), then it seems we are unable to have our custom fields or entities installed as part of a site deployment.
Or are hook_uninstall() and hook_install() run twice? Once during config:import and once during some other part of the deployment process?
Very confused here as this requirement to not make changes to entities in hook_install seems to run counter to the whole point of the hook.
Full migration tests were run. Everything checked out.
Thank you @markdorison and @rajan-kumar2026. In addition to the suggestions Mark is making, I think the point of this ticket is to figure out if we want to support Drush 12 or not. Currently, our drush commands are not compatible with Drush 12. So if we are going to add
"drush/drush": "^11 || ^12 "
we are basically saying we support drush 12, when we _don't_ support drush 12 (yet).
If we are going to add `^12` we will have to rewrite a lot of code in this module so that it can still output to the console. It's a big job.
apotek → created an issue.
apotek → created an issue.
I feel it might be worth adding a section on limitations to this. For example, you can't set the priority of your overrides. A use case I tried to solve for might illustrate: Writing a drush command where one of the options passed to the command is intended to override the configuration, but just for the run of that drush command. I can create the override, and add the override to my config factory, but (and here come the limitations):
- The override does not change the global state of the configuration unless it is loaded as service. If you are setting the override in code, only your exact class and following code has access to the override, in which case you are getting no benefits from doing the override at all. In other words, you _can_ make an override dynamically, but it is only available to your current execution scope. A config factory in another class will not pick it up.
- You cannot override any config that might be set in settings.php. Since you can't control what users might set there, you effectively cannot have any assurance that your override, even when loaded as a service, will have any effect at all.
In case of failed validation, the documentation here, as elsewhere, explains that
> by default the return will be a 404 page.
but I can find no documentation on how to change that default response type. The structure of routes → overview page mentions nothing to this effect.
In the case of a malformed value, for example, a developer might prefer to follow the standard and issue an HTTP Bad Request (400) response instead of a possibly misleading 404 Not Found.
But I can see no way to do this.
Suggestions:
1) If there *is no way* to change the response type, the documentation should be updated to say:
- In the case the validation fails, by default the return will be a 404 page.
+ If validation fails, a 404 Response will be issued. If you want to respond with another HTTP response, you cannot use this form of validation and will need to implement your own within your controller.
2) If there *is* a way to change the default response type, can we add that documentation here?
Thank you.
Following the exact instructions on the home page (for version 5). I put this in my composer.json libraries section:
{
"type": "package",
"package": {
"name": "enyo/dropzone",
"version": "5.9.3",
"type": "drupal-library",
"dist": {
"url": "https://github.com/dropzone/dropzone/releases/download/v5.9.3/dist.zip",
"type": "zip"
}
}
}
I run `composer require drupal/dropzonejs enyo/dropzone` and I get the exact output mentioned by @kopeboy in #4.
My previous installation had `^5.7` as the enyo/dropzone constraint. I also tried requiring "enyo/dropzone:^5.9" and the same response came out: "Root composer.json requires enyo/dropzone ^5.9, 5.7.2, found enyo/dropzone[5.7.2] but it does not match the constraint"
In the end, while I can update drupal/dropzonejs to latest, I am unable to update enyo/dropzone to 5.9.3 and am stuck on 5.7.2. This is not breaking the site, but obviously it is less than ideal. What is interesting is that dependabot keeps trying to update enyo/dropzone to 5.9.3 and I get pull requests for an updated composer.lock with the new 5.9.3 path in it, but my builds fail of course when my CI tries to build the dependabot PRs.
Seems like there is a hangup somewhere that I certainly cannot identify. --with-all-dependencies does not fix this, nor does a relaxation of minimum stability.
Thank you @fjgarlin, @berdir for your patient replies. Many of us mortals do not have the ability to monitor slack daily and stay on top of every META issue. It's hard enough to just (fail) at keeping up with bug fixes and feature requests. I accept that the cost of not participating on Mt Olympus is that our fate must be handed to us.
I just feel in the past year that the balance of work done to develop actual features or fix bugs is reaching less than half, and the other more-than-half is just keeping up with deprecations, Symfonyisms, phpstan and linters. That's what I am seeing in the issue queues. Thanks again for the responses. I'll try to stay on top of META developments in the future.
Is there anyone who takes the time to assess how these decisions affect contrib developers. A person makes a suggestion, there's no meaningful debate, and the next thing you know, every maintainer of every contrib module now has to spend time making cspell happy. When code linters came out I was an early adopter. Now I see merge request after merge request that add nothing to the functionality of the modules, but merely strive to keep them compliant with the latest enforcement tools. And... in the comments on contributed fixes, more is made of an linter warning on a yml indentation, that on how the code is actually working.
I am waiting for someone to roll out a linter that enforces the correct use of the oxford comma.
Where is this stuff being talked about? And is it ok to push the same core procedures out onto all contributed module developers without any meaningful debate at all? (I apologize if there is such a forum, in which case, I would love a link to that discussion).
Merge request is up:
We still get this notification:
[warning] Unable to find migrated content having “JF11978345” as a source id while processing the deletion queue.
But the queue item is released from the queue….
select * from queue where data like ‘%JF11978345%’;
[nada]
So it will not be run again and again ...
apotek → created an issue.
Added relationship to issue #3084967, which has the solution.
> I can reproduce the issue, but setting "Needs work" since there's open threads on merge request.
Keeping this as reviewed and tested. I am not sure why gitlab says there are three unresolved threads. The "resolve thread" buttons are not visible, so while it indicates that all threads are unresolved, it appears that they actually were resolved at some point in time. In any case, all questions have been resolved with decisions or changes.
This looks good to go.