fjgarlin → created an issue.
Then, in my opinion, this issue in this queue should be closed as there is already a custom image being used in 📌 [CI] Use custom image and improve caching for Cypress Active , which I think it's better suited for XB.
Yeah, having an official image would only make sense if cypress will be used for core too.
Custom image seems better rather than official one imo, unless we are planning to include cypress testing in core.
Will cypress testing make it to core?
Switching this made a huge difference in Experience Builder total run time.
📌
Improve CI pipeline runtime
Active
Re your last commit on this issue's MR, you could have maybe played with the variable CI_PIPELINE_SOURCE, so it does not trigger on MR but it does on push to the main branch. You could have also set the "when: manual" so the jobs can be triggered manually.
I made a comment on that other issue. That'll defo shave more minutes.
Using custom images is a great way to speed things up in the CI system.
Some documentation can be found here: https://project.pages.drupalcode.org/gitlab_templates/info/customization...
This is merged now. Big big thanks!
Feel free to reopen and reuse this issue for further MRs or to do it in a follow up. I'm happy either way.
Yeah, on a bigger set of tests it makes a big difference to run tests with run-tests.sh. Amazing progress so far!!
The GitLab template must have improved quite a bit since I last looked at it
We've been busy 🙂
Another possible improvement could be to only run one of the DBs in the matrix during MRs and make the other jobs manually triggered. I guess most aren't dealing with different database nuances. Then we can leave the main project branch and/or scheduled runs to check all databases.
There is an error in the pipeline, but I don't think it's related to the changes.
larowlan → credited fjgarlin → .
This is great so far! Give that it's an internal check and mostly just moving code, we can merge whenever you are happy with it (I am). If working on the second level is going to be quick and easy, we can do it here, otherwise I'm happy to merge this as is, and then do the second level in a different MR.
I'll let you decide what you think is best here.
He actualizado el issue summary con las cosillas que hay que ir haciendo.
I'm happy with 1 and 2 and also not surprised at having that many as we've been creating new jobs without thinking of the order of the lines, so this issue is a great step into standarising that. We don't need to go 1:1 with Tin's suggestion (as you propose in 1), we just need to setup a logical standard and then follow it.
and which I have replicated in the comments in the code here.
I've seen the commends on the code and they are great!!
From my point of view, the script that checks the order is in a great position, so we just need to move more elements in the two main files.
The second MR https://git.drupalcode.org/project/drupalorg/-/merge_requests/337 is mostly ready, but it relies on MR336 being merged first together with the deployment steps for that part.
After that, and when this is merged, we should be able to switch back and forth between the "legacy" and "modern" credit system with the following commands:
Use modern system:
drush vset drupalorg_credit_system 'modern'
drush cc views
Switch back to legacy system:
drush vset drupalorg_credit_system 'legacy'
drush cc views
A third part, and once the new system is stable and in use, will be to clean up the code from the legacy system and remove the switch, but we will go one step at a time.
It looks great so far. I'd just change the comment mentioned in the MR slightly to make it more obvious about why that part is there, but other than that (which is really minor) we can start changing the "main" and "main-d7" file. Setting to NW based on that.
Merged. Thanks!
Assigning credit.
That's a great workaround, especially when the pipeline doesn't run certain jobs like "nightwatch", "cspell", "stylelint" which actually need those files. But if your pipeline don't need it then this is a good workaround.
But as mentioned, there are some steps that rely on the node_modules folder being present, so I'd say that this keeps on working as expected. We are happy to help debug the projects that have this issue. Most of the time is bringing full git history, and in this case, we could further reduce removing that folder from the artifacts. Thanks for sharing!
There is a big migration underway right now and during this week. This will go right after.
The issue is the same. Bringing nearly 1000 commits might take a lot of space on some repos. Do you need the full git history of a package when you only need a version of it?
Can you try syntax like this in your composer.json file?:
"mably/slick": {
"type": "package",
"package": {
"name": "mably/slick",
"version": "1.8.2",
"type": "drupal-library",
"dist": {
"url": "https://github.com/mably/slick/archive/refs/heads/topic/jquery4.zip",
"type": "zip",
}
}
}
This is now merged. I'll start running the migrations in production shortly. Thanks!
That might be a separate issue. The change in this MR is purely for the "Fatal error" issue.
Agree, let's see how much moving around it is and take it from there. Once we do the first big "moving", then keeping it consistent will be easier.
I left some comments in the MR.
As for the ".gitlab-ci.yml" being ignored by the tool. It seems to be the default. I did: npx eslint --debug .gitlab-ci.yml with an empty .eslintrc.js file and I still got 0:0 warning File ignored by default. Use a negated ignore pattern (like "--ignore-pattern '!<relative/path/to/filename>'") to override.
https://github.com/eslint/eslint/issues/12938
https://github.com/eslint/eslint/issues/16265#issuecomment-1344037580
I agree that the workaround is simple enough to not do anything in the templates. In any case, I am assigning credit as this is a very valid question and an equally good reply/workaround.
The MR and results look good to me. All your tests and the downstream pipelines look good, so marking this RTBC.
🙂
I knew perfectly what you meant by that, I usually do these type of searches as well and then need to iterate to see the detail, so this is really helpful to see the context straightaway.
The MR is ready here:
- Need to pull the latest version of the drupalorg site and run ddev drush deploy
- Then you can make sure the code on this MR is in place
- Then run the migration with the update flag: ddev drush migrate:import drupalorg_migrate_project_general --update
fjgarlin → made their first commit to this issue’s fork.
Fields added to the new site (private repo for now):
- Composer types: https://gitlab.com/drupal-infrastructure/sites/drupalorg/-/commit/b19914...
- Project categories: https://gitlab.com/drupal-infrastructure/sites/drupalorg/-/commit/7e7ebd...
The rest will be done in the migration issue.
Merged!
Thanks for the changes. I'm fine with not having quotes if it's valid syntax.
Rest of the downstream pipelines: https://git.drupalcode.org/issue/gitlab_templates-3518579/-/pipelines/47...
Everything seems ok and it should not change anything really in the outcome of jobs, so I'll merge this shortly.
Good improvements here. Thanks for working on them.
Added a configurable way to throttle the requests to the new system, in case we need it.
Simplified deployment instructions.
fjgarlin → created an issue.
The MR looks good to me. Merging and it will be deployed in the next site deployment.
This is now merged.
It looks perfect. Thanks for bringing these variables for these jobs. Merged!
Yeah, I think it's only possible to pass down global variables, not per job.
KeyCDN: https://git.drupalcode.org/issue/gitlab_templates-3517992/-/pipelines/46...
The MR looks good to me and the documentation additions are great too. Marking RTBC unless you were expecting to run any more tests.
Great, this is ready. RTBC. Thanks!
Remember that we can pass variables to the downstream pipelines from the current MR, so you can test the pages argument with "keycdn".
The MR looks good already. Once we have a test for each we can set RTBC.
Downstream pipelines: https://git.drupalcode.org/issue/gitlab_templates-3517992/-/pipelines/46...
And no reason for the backticks in the "description" values other than we did it like that, but happy to change to double quotes if needed. We have yml validation so it should be safe to do. Tho there are a lot of places where we use them, so maybe on a separate follow-up issue to isolate features.
First part: Send data to new.drupal.org:
- MR: https://git.drupalcode.org/project/drupalorg/-/merge_requests/337
- Settings on D7:
-- Set drupalorg_token to the same value as the one set in D10
-- Setup queue drupalorg_issue_events
- Settings on D10:
-- Allow 'contribution_records' in drupalorg_allowed_content_types
-- Allow /contribution-record* paths on Fastly
-- Setup environment variable so this line works: $config['drupalorg.settings']['token'] = getenv('DRUPALORG_TOKEN');
-- Setup queue contribution_records_import_queue_worker
I will try to split the MR into two:
- One for pushing data to D10
- One for reading that data from D10
Good suggestion. Yeah, let's go ahead with _MKDOCS_EXTRA as that's the tool we are using for that job.
Aren't those references to release nodes? I think the migration IDs need to be different in that case. I made some suggestions in the MR.
Those are nice additions too, thanks! Also, the refactoring of the variable makes the comment easy to follow, so I'm happy with that.
Rest of downstream pipelines: https://git.drupalcode.org/issue/gitlab_templates-3514999/-/pipelines/46...
Re #14, not sure what can be the reason for the mixed results. I think core suffers from similar things. I don't think that it's anything new that we might be introducing here, but something related to the driver service.
Once you've run the last round of tests that you mention, I think we can remove the debug code and this should be good to go. Great job!
Merged.
fjgarlin → changed the visibility of the branch 7.x-3.x to hidden.
I think that the release references should use the migration_lookup plugin to double check that they belong to migrated releases.
I've also migrated the labels to the new Drupal site so when we migrate they show with these new ones.
Not worth it. This was a POC so we can close and re-open if needed in the future.
The fact that it's just max php jobs might mean that the issue is at image level. Tho the database version seems to be the same one for all the variants, which makes it weirder.
Thanks for the rebase. Rest of downstream pipelines: https://git.drupalcode.org/issue/gitlab_templates-3463044/-/pipelines/46...
Merged.
Not sure why I didn't close it in #23, but yeah, I think we should close it.
Yeah, agree that we don't need anything for D7.
I made a question in the MR about a small refactoring, but I'm not sure if it's like that intentionally. NW based on that question and possible refactoring.
RTBC. The shorter syntax was an alias.
Registry images: https://gitlab.com/drupal-infrastructure/drupalci/drupalci-environments/...
Dockerfile inside the folders (the path is used to generate the image name): https://git.drupalcode.org/project/drupalci_environments/-/tree/dev/webd...
I’ve seen this in action and it’s really powerful yet simple to implement and easy to understand, so at the very least trying to solve the problem of the dependencies would unlock POCs and further proposals.
+1 fwiw
Doh!! it's just the "echo" 🙈
Ignore that then please and sorry for the noise.
Yup, it's that code there.
I thought we were fixing the path to line up with another automatically generated path. Is that commit not doing that?
The changes look good, but we are also changing the path of the generated files in here, not just creating empty ones.
Marking it RTBC, but can we change the issue summary with the full proposed resolution?
I'm happy either way. On one hand it makes sense two issues as they are easy to isolate and it'll be clear what the changes are, but on the other we only created that new variable because we needed it for this issue, so it's a kind of nice side effect of it.
I'd say let's leave it all in one issue, as it doesnt' really need extra documentation.
I suggested a name change as it was too close to DRUPAL_CORE.
x.y.z - I usually bump "z" for bug fixes and maintenance tasks. And I usually bump "y" for new features/bigger changes.
I was about to include it as part of 1.8.1, as this could be considered a bug, but I also thought that it was a big-enough change to bump the minor. I kind of followed https://project.pages.drupalcode.org/gitlab_templates/help/contributing/...
This is now merged in core. We will need to check if the issue still happens here.
I see you gave it a go at 📌 Migrate project_release_supported_versions table Active .
Merged! Thanks so much for the MR and additional tests.
Images were pushed to our gitlab.com registry as part of
📌
CI: Push to gitlab.com registry instead of dockerhub (avoid rate limit errors)
Active
.
Core also made the same changes in
📌
CI: Switch drupalci image registry from dockerhub to gitlab.com (mitigate rate limit errorss)
Active
.
I will merge this now and make it the default for all contrib.
MR ready.
fjgarlin → created an issue.
Oh, thanks for letting us know. Hopefully it'll be fixed upstream then.
I've merged the changes. Fixed!
@drumm - for the migration, is it just a table-to-table migration?
Tests are passing now and code looks good. The change will be very similar for contrib.
RTBC.
I think an alias was created somewhere from the old naming to the new naming for chromedriver.
Code-wise, it looks good, and the downstream pipelines are running as expected and pulling the images from the new registry.
Triggering downstream pipelines (there are a few, which is great for testing): https://git.drupalcode.org/issue/gitlab_templates-3516958/-/pipelines/46...
Agree, we can close and keep using MR8 as needed.
Thanks!
Tested with the following CSV:
1,speaker
2,speake
fake-user@fake-email.com,organizer
drumm,volunteer
b_man,organizer
Then ran it for event https://fjgarlin-drupal.dev.devdrupal.org/community/events/midcamp-2025-...
Like this drush drupalorg-event-volunteers 3443832 ./test.csv -v
$ drush drupalorg-event-volunteers 3443832 ./test.csv -v
Adding user 1 as field_event_speakers. [notice]
Adding user 2 as field_event_volunteers. [notice]
Adding user 3064 as field_event_volunteers. [notice]
Adding user 2689807 as field_organizers. [notice]
Saving event. [notice]
The following users could not be matched: fake-user@fake-email.com.
fjgarlin → created an issue.
I don't think we need to show this variable or add it to the documentation. It was a regression from Nightwatch which will likely be fixed. Also, it's an "internal" variable.
The code looks good to me, thanks!
This is a great find!! I'd say let's add the variable to the nightwatch base job regardless of the outcome of that issue.
Do you want to give it a go?
There seems to be a lot of unpredictability in pipelines at the moment.
Yes, dockerhub needs to mark our account in a certain way so we don't hit limits. This was sudden, unexpected and Tim (@hestenet) is dealing with that atm.
Maybe if run-tests.sh has a long future, which I presume it does, we could ask for that enhancement?
At the very least, it's worth opening an issue in core to discuss it.
The changes on this MR look good, so I'm marking it RTBC.