πŸ‡¦πŸ‡Ή Vienna
Account created on 19 March 2008, over 17 years ago
#

Merge Requests

More

Recent comments

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

The subquery is needed because a JOIN to itself can also be slow when you have many revisions. I analyzed that in πŸ› Entity queries querying the latest revision very slow with lots of revisions Needs work where this regression was introduced.

We have 2 problems I see:
* The self join is very bad for entities with thousands of revisions
* The subquery is very bad for sites with millions of entities

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

Coder 9.0.0-alpha1 released, but I plan to check some more sniffs we could remove by upstream sniffs. https://www.drupal.org/project/coder/releases/9.0.0-alpha1 β†’

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna
πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

Merged.

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna
πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

klausi β†’ created an issue.

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna
πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna
πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna
πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

klausi β†’ created an issue.

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna
πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

Merged.

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna
πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

klausi β†’ created an issue.

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

Coder 8.3.31 released with this fix!

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

Thanks, can I also get credit on this issue? I adapted Coder over the last 10 years to support what drupal core needs 😊

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna
πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna
πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

klausi β†’ created an issue.

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

The biggest chunk of work was done in πŸ“Œ Update php_codesniffer to 4.0 Needs work .

I started to replace some sniffs with sniffs from Slevomat, which is nice to reduce the code to maintain in Coder.

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

Merged.

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna
πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna
πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

Tagging.

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

Looks good to me, thanks!

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

I would like to keep Drupal 7 support as I'm still running lots of Drupal 7 sites and use Coder as part of the dev process :)

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

Tagging.

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

I committed the work you started to 9.x, continuing at https://github.com/pfrenssen/coder/pull/272

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna
πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

klausi β†’ created an issue.

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

Merged https://github.com/pfrenssen/coder/pull/270 . Used the wrong issue number in the commit to πŸ› Typo in WidgetInterface.php Active , but should not matter.

I tweaked the regex to allow 3 dots at the end and only target 2 dots.

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

tagging.

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

Merged, thanks!

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

As this coding standard should apply to all Drupal code we should do the same Slevomat sniff with config in Coder's ruleset.xml file.

Core is getting out of sync with Coder rules again ... please always make pull requests against Coder as well when you change core's phpcs.xml.dist file, so that we have the same standards everywhere.

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna
πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

I'm open to pull requests at https://github.com/pfrenssen/coder if you want to unify this.

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

Thanks, approach makes sense. Left one minor comment on the PR https://github.com/pfrenssen/coder/pull/268

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

Hey thanks!

I think we should probably make a new Coder 9.x branch when we adopt this major upgrade.

Feel free to continue working in a pull request again 8.3.x in the meantime. We do pull requests on Github, please open one against https://github.com/pfrenssen/coder

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

Thanks, merged!

Usually I try to make releases every 3 months if changes were merged in that period.

I think I can make a new release at Drupalcon on contribution day next Friday.

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

Thanks, changes look good to me, the only thing missing is now the test fixing.

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

Merged!

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

klausi β†’ made their first commit to this issue’s fork.

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

Merged, thanks!

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

merge request ready.

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

klausi β†’ created an issue.

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

This change caused a critical performance regression, the query is factor 1000 slower on a big site with 2 million paragraph entities.

You can find my analysis in the critical follow-up issue πŸ› Updating to 10.5.3 causes gateway timeouts on revisioned content Active .

We have 2 problems I see:
* The old query is very bad for entities with thousands of revisions
* The new query is very bad for sites with millions of entities

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

For comparison the reverted query, it takes 0.2ms to run (factor 1000 faster):

SELECT base_table.revision_id AS revision_id, base_table.id AS id
FROM
paragraphs_item_revision base_table
LEFT OUTER JOIN paragraphs_item_revision base_table_2 ON base_table.id = base_table_2.id AND base_table.revision_id < base_table_2.revision_id
INNER JOIN paragraphs_item_field_data paragraphs_item_field_data ON paragraphs_item_field_data.id = base_table.id
WHERE (base_table_2.id IS NULL) AND (paragraphs_item_field_data.id = '6091304')

EXPLAIN output showing that an index can be used for all parts:

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

Having an unlimited SELECT subquery is not a good idea, this resulting query for 2 million paragraphs items takes 2 seconds to run:

SELECT base_table.revision_id AS revision_id, base_table.id AS id
FROM
paragraphs_item_revision base_table
INNER JOIN paragraphs_item_field_data paragraphs_item_field_data ON paragraphs_item_field_data.id = base_table.id
WHERE (base_table.revision_id IN (SELECT MAX(base_table.revision_id) AS expression
FROM
paragraphs_item_revision base_table
GROUP BY base_table.id)) AND (paragraphs_item_field_data.id = '6091304');

EXPLAIN output shows the subquery as the only part that does not use a DB index:

Uploading the revert for 10.5.x as stable patch file for composer patches.

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

Thanks! Can you open a PR at https://github.com/pfrenssen/coder/pulls ? We are still doing the pull requests there for now.

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

Merge request: https://git.drupalcode.org/project/feeds_http_auth_fetcher/-/merge_reque... (seems broken, not sure why)

Attaching stable patch for composer patches.

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

klausi β†’ created an issue.

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

Here is an additional update function to switch the DB schema to blob for existing sites.

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

Thanks, merged!

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna
πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

klausi β†’ created an issue.

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

GraphQL 5.0.0-beta1 released, please test!

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

Nice, thanks!

Uploading stable patch file for composer patches.

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

Sorry, this was merged after alpha3, please test the dev version.

I will make a new release soon as well.

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

Ah sorry, it was merged, just the commit does not show up here. Best include the issue number in the commit message next time.

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

This issue was not fixed yet, right? Let's leave it at RTBC until the fix is merged and committed.

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

typos

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

started preparing the API changes for the 5.0.0 release notes.

Should probably be moved to a change record at some point.

I'm planning to release a first beta version soon, thanks a lot for testing the alpha release (2 issues on that fixed already in 5.x dev).

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

Merged the small fixes in the pull request, thanks @almunnings!

I'm not sure that this will fix the cache issue here, please test if this helps.

Then I can make a new 4.x release soon.

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna
πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

Merged.

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

merge request created.

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

Merged that one already, let me know if you still see a problem!

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

This should do it, please test!

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna
πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna
πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

Aha good point, I should have checked Drupal core plugins.

Is it enough to just add this property to the attribute classes? Or do we have to do anything on the plugin managers as well? Probably not, as you say the property is defined on the parent Plugin attribute class.

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

Thanks for reporting!

I had the same problem in our installation because the 2 int properties were missing in our graphql entity server config:

query_depth: 20
query_complexity: 0

I wrote a custom update function to work around that:

/**
 * Set the missing query_depth and query_complexity values.
 */
function jobiqo_graphql_update_10103(): void {
  \Drupal::configFactory()
    ->getEditable('graphql.graphql_servers.jobiqo_schema')
    ->set('query_depth', 20)
    ->set('query_complexity', 0)
    ->save();
}

The int values were introduced in πŸ› 5.x fatal error creating new severs Active .

We have 2 options to solve this:
1. Implement constructor on the Server entity and always cast those 2 to int
2. Write an update function to update all graphql server entities that miss those 2 properties, where they are not int.

I'm inclined to go with option 2, so that we don't have to maintain workaround code in the constructor.

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

GraphQL 5.0.0-alpha3 released!

Mostly refactoring to use PHP attributes and some bug fixes.

Thank you Daniel Bosen, Al Munnings, Andrii Aleksandrov and Ben Stallings for contributing fixes!

https://www.drupal.org/project/graphql/releases/5.0.0-alpha3 β†’

I think we can do the first beta soon, let me know of any blockers you see.

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

Thanks, merged!

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

Thanks, merged!

Not the best test as it would be better to test a real entity, but I think we don't care.

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

merged.

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

Thanks, I think we need to expand the test coverage to test with string and TranslatableMarkup for the description.

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

Ah ok, I don't think we need a functional test, we can just use a real entity. Can you create a node in the test and use that instead of a mock? We have a database connection available in our kernel tests.

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

Thanks, that is surprising! Can you add a test case? Looks like test coverage was missing before, but we should ensure the code is tested.

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

Pull request ready.

This was the final prompt for AI: "convert the doc block annotation to php attributes. use statement: "use Drupal\graphql\Attribute\DataProducer;". For ContextDefinition use Drupal\Core\Plugin\Context\ContextDefinition. keep the top doc comment sentence. use named parameters for all ContextDefinition constructors, the data type property is called "data_type". Use TranslatableMarkup for @Translation. make sure the use statements on top of the file are sorted alphabetically. for each property of ContextDefintion use a new line. If the data type of a context definition starts with "entity:" then use the class EntityContextDefinition with use statement Drupal\Core\Plugin\Context\EntityContextDefinition. If the data type of a context definition is exactly "entity" then use the class ContextDefinition with use statement Drupal\Core\Plugin\Context\ContextDefinition. Make sure that all multiline array elements have a comma at the end. Ensure that all context definitions in "consumes" are present in the attribute conversion and have the same name. the number of entries in "consumes" needs to be the same. If the class already has an attribute then skip the file."

I took me 5 hours to create this pull request. Was I really faster with AI? Maybe a bit, but fiddling and reviewing changes also takes time. This might be a typical case where a developer thinks they are more productive with AI and suddenly 5 hours have passed.

At least I enjoyed the refactoring more as I did not have to type out or copy/paste all the modifications. I would estimate that 95% of the changes the AI created correctly and the 5% rest I had to fix manually.

I'm very thankful for the automated tests, which caught a lot of small mistakes that I then had to feed back into the prompt.

I used Github copilot in agent mode with model Gemini 2.5 Pro.

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

klausi β†’ made their first commit to this issue’s fork.

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

Merged.

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

klausi β†’ made their first commit to this issue’s fork.

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

Merged.

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

klausi β†’ made their first commit to this issue’s fork.

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

Merged.

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

klausi β†’ made their first commit to this issue’s fork.

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

Nice, thank you!

Merged.

I will try to convert the annotations to attributes with AI, as the Drupal rector tool did not work for me. Issue: πŸ“Œ Use attributes to define DataProducer plugins Active

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

Merged.

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

Is the rector code supposed to work with contrib plugin annotations and attributes supposed to work? Tried to convert graphql dataproducer plugins, but I get no output.

Rector config:

<?php

declare(strict_types=1);

use Drupal\graphql\Attribute\DataProducer;
use DrupalRector\Drupal10\Rector\Deprecation\AnnotationToAttributeRector;
use DrupalRector\Drupal10\Rector\ValueObject\AnnotationToAttributeConfiguration;
use Rector\Config\RectorConfig;

return static function (RectorConfig $rectorConfig): void {
    $rectorConfig->ruleWithConfiguration(AnnotationToAttributeRector::class, [
      new AnnotationToAttributeConfiguration('10.0.0', '10.0.0', 'DataProducer', DataProducer::class),
    ]);

    if (class_exists('DrupalFinder\DrupalFinderComposerRuntime')) {
        $drupalFinder = new DrupalFinder\DrupalFinderComposerRuntime();
    } else {
        $drupalFinder = new DrupalFinder\DrupalFinder();
        $drupalFinder->locateRoot(__DIR__);
    }
    $drupalRoot = $drupalFinder->getDrupalRoot();
    $rectorConfig->autoloadPaths([
        $drupalRoot . '/core',
        $drupalRoot . '/modules',
        $drupalRoot . '/profiles',
        $drupalRoot . '/themes'
    ]);

    $rectorConfig->skip(['*/upgrade_status/tests/modules/*']);
    $rectorConfig->fileExtensions(['php', 'module', 'theme', 'install', 'profile', 'inc', 'engine']);
    $rectorConfig->importNames(true, false);
    $rectorConfig->importShortClasses(false);
};

Invocation:
vendor/bin/rector process modules/graphql --dry-run

output:

klausi@drupal-11-web:/var/www/html$ vendor/bin/rector process modules/graphql
 36/68 [β–“β–“β–“β–“β–“β–“β–“β–“β–“β–“β–“β–“β–“β–“β–‘β–‘β–‘β–‘β–‘β–‘β–‘β–‘β–‘β–‘β–‘β–‘β–‘β–‘]  52%
In ProcessPool.php line 30:
                                   
  Process "qabrfesumt" not found.  

Looks like rector is crashing here?

Did anyone convert contrib annotation successfully with an automated tool?

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

klausi β†’ created an issue.

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

klausi β†’ created an issue.

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

Merged, thanks!

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

Merged!

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

Thanks for reporting - I would like to take a slightly different approach by refactoring this config setup into a common method on the schema plugin manager.

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

klausi β†’ made their first commit to this issue’s fork.

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

Resolved merge conflict, uploading stable patch file for composer patches.

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

klausi β†’ made their first commit to this issue’s fork.

πŸ‡¦πŸ‡ΉAustria klausi πŸ‡¦πŸ‡Ή Vienna

Merged in the 2.0 branch and resolved conflicts.

Uploading stable patch file for composer patches.

Production build 0.71.5 2024