🏄‍♂️🇦🇺Sydney, Australia
Account created on 24 September 2008, about 17 years ago
  • Co-Founder and Technical Director at PreviousNext 
  • Co-Founder and Technical Director at Skpr 
#

Merge Requests

More

Recent comments

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

kim.pepper made their first commit to this issue’s fork.

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

I think it's fine to move it once ai_search is in its own module. It's pretty basic, and we aren't actually using the value objects anywhere yet.

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

Updated IS

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

Updated the title and IS to reflect the new approach.

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

Testing in PHPStorm this seems to work well.

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

Added the conditional return type as per #21

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

Added a new MR that just adds the class-string typehints to \Drupal::service().

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

Could we merge this docblock with the existing \Drupal::service() docblock somehow?

*  @template T of object
*   
*  @param class-string<T> $class
*
*   @return object&T
*       A service that is an instance of $class.
🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

I would prefer a way for users to add support for additional units rather than hard coding to what is in the spec.

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

@gxleano you don't need to manually close issues. Fixed issues get automatically closed after 2 weeks (?).

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

From the discussion linked above https://github.com/design-tokens/community-group/issues/245

I think a similar approach would be a better way to address prototype/pre-spec implementations. The spec can allow parsers/transformers to ignore invalid tokens. If a vendor wants to add a unit that isn't in the spec yet, they just use it in the unit property as if it was valid and then end users can use a plugin or custom transform (perhaps written by the vendor) in the parser to handle it.

So just because it's not in the spec, doesn't mean we can't support the units we want in our implementation.

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

Not sure if we need to do anything with \Drupal\file\Validation\Constraint\UploadedFileConstraint ?

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

This got stalled most likely because it was trying to do too much.

I'm starting to split things off into smaller issues starting with 📌 Add a UploadedFilesExtractor and remove duplicate code Active

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia
🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

kim.pepper created an issue.

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

Published the CR

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

Squashed commits and rebased on 11.x

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

Rebased this on 2.0.x and copied over the changes that were made to AiVdbProviderClientBase in the mean time.

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

Leaving this open but marking PMNMI for any more feedback. Otherwise this will be closed as Won't Fix as per #4 in the near future.

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

kim.pepper created an issue.

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

OK removed.

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

Deprecated cache.field_config_memory and replaced with cache.memory.

Do we need to deprecate? or can we just remove it?

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

Coming from the duplicate issue 📌 Add return types for all methods with a rector rule Active here is a basic rector.php config for adding return type hints.

<?php

return Rector\Config\RectorConfig::configure()
  // path from composer root.
  ->withPaths([__DIR__ . '/modules/custom/search_api'])
  ->withTypeCoverageLevel(10);

We can add the #[ReturnTypeWillChange] attribute to suppress warnings in implementing classes.

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

I'd like to opt-in https://www.drupal.org/project/ai_vdb_provider_opensearch as it's a small project that shouldn't disrupt anyone too much.

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

My 2 cents... if we are injecting services at discovery time, what advantage does this have over regular container services?

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

kim.pepper changed the visibility of the branch 3536726-use-callableresolver-for to hidden.

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia
🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

Committed to 1.x. Thanks!

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

kim.pepper made their first commit to this issue’s fork.

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

Can you confirm the version (or commit hash) you are using?

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

OK. I think this is ready for reviews again.

I went down the path of trying to add CallableResolver to entity form constructor and every sub-class constructor. I think that would be very disruptive. All the unit tests would need to be updated etc. I ended up with checking if the container was available.

Not ideal that Entity form calls a service as it should be a value object like other forms.

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

Updated the title to be more specific.

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

I spent a couple of hours looking at how we could use CallableResolver and avoid calling it in FormState.

I deprecated \Drupal\Core\Form\FormStateInterface::prepareCallback() and looked at where it was being called from.

Obvously a lot of test fails 😬 but could be a good first step.

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

Rebased on 11.x

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

Rebased on 11.x

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

Based on the feedback, I've updated our Time service to implement Psr\Clock\ClockInterface and added a service alias for @datetime.time

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

Still NW for failing tests.

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

Committed to 2.x and 3.x. Thanks!

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

kim.pepper created an issue.

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

Deleted the update test as we are no longer updating.

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

I made the post update a no-op.

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

Tests are still failing so I left at NW

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

If we are mocking the current time I would much prefer us to use Add PSR-20 Clock Interface as composer dependency Active and the follow-up Implement PSR-20 in datetime.time service Active

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

Marked as "Closed (duplicate)".

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia
🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

Committed to 2.x. Thanks!

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

Committed to 3.x and 2.x. Thanks!

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

Seems like a sensible fix. The phpstan errors are because the baseline format has changed slightly so I regenerated it.

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

Rebased on 11.x and removed the post-update hook as per #11

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

Rebased on 11.x so back to RTBC

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia
🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

NW for linting failures.

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

Updated deprecations message in two constructors (DirectoryWithMetadataDiscovery and DirectoryWithMetadataPluginDiscovery to be for removal in 12.x.

I think I'm ok to put back to RTBC in this case.

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia
🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

This is ready for reviews now.

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

Testing this locally, I am getting

illegal_argument_exception: Can't update non dynamic settings [[index.analysis.filter.synonyms.type, index.analysis.analyzer.default.filter, index.analysis.filter.synonyms.lenient, index.analysis.analyzer.default.type, index.analysis.analyzer.default.tokenizer, index.analysis.filter.synonyms.synonyms]] for indices [content]

when saving mappings.

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

Trying a different approach with a aoss mode setting.

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia
🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

Looks like we will need to sort out Compatibility with AWS Open Search Serverless (AOSS) Active before we can properly test this. Postponing on that.

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia
🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

Re #10 this was done in 📌 Convert models to plugins, remove hardcoded model string checks Needs review and merged to 1.x.

I have rebased this MR on 1.x

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

📌 Convert models to plugins, remove hardcoded model string checks Needs review is in so this is unblocked.

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

kim.pepper created an issue.

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

Committed to 1.x. Thanks!

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

There's a long list of models. I guess people can create their own plugins if they want a model we don't include here? https://docs.aws.amazon.com/bedrock/latest/userguide/models-supported.html

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia
🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

Great improvement. As discussed in slack we can deprecate titan v1 and add titan v2 in the follow-up 📌 Deprecate titan text embedding v1 and add v2 Closed: duplicate

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

kim.pepper created an issue.

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia
🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

kim.pepper created an issue.

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

Committed to 1.x.

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

Committed to 3.x and 2.x

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

kim.pepper created an issue.

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

Getting this error locally and it's failing the test:

Exception: Warning: Undefined array key "connector"
Drupal\ai_vdb_provider_opensearch\Form\OpenSearchConfigForm->validateConnectorConfigForm()() (Line: 125)

That looks like the trait in search_api_opensearch

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

kim.pepper created an issue.

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

Closing this as duplicate.

As well as committing #3545869: Add a ConnectorFormTrait for re-use I have commited the provider code here to the 1.x branch of https://www.drupal.org/project/ai_vdb_provider_opensearch in 📌 Use the official opensearch-php library Active

Please continue with remaining work (such as adding tests!) in https://www.drupal.org/project/ai_vdb_provider_opensearch

All contribution credits have been added to both issues.

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

Commited to 1.x. Copied commit credits from 📌 Add support for AI module VDB Provider Active

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

Updating title and IS

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

Committed to 3.x and cherry-picked to 2.x.

Added credits from related issue 📌 Add support for AI module VDB Provider Active

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia
🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

kim.pepper created an issue.

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

No this is passing, I'm going to split of an issue to just add the trait to the core module for re-use.

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

Responded to suggestions. I don't think we should be adding new interfaces in this issue just for the sake of autowiring.

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

Committed to 2.x

There is currently a bug and I can't save contribution records, but I will come back and add them when it's fixed.

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

kim.pepper made their first commit to this issue’s fork.

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

Committed to 2.x

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

Rebase on 11.x

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

kim.pepper created an issue.

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

I added DeprecatedServicePropertyTrait for the removed $fileSystem property.

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

Some example code:

Create a plain old value object with some constraint attributes:

use Symfony\Component\Validator\Constraints as Assert;

class Embedding {

  public function __construct(
    #[Assert\Regex(
      pattern: '/^[A-Za-z0-9_-]+:[A-Za-z0-9_-]+$/',
      message: 'Id should be in the format "prefix:suffix"',
    )]
    readonly public string $id,
    #[Assert\NotBlank(message: 'Values should not be empty')]
    public array $values,
    #[Assert\NotBlank(message: 'Metadata should not be empty')]
    protected array $metadata,
  ) {}

Create a Symfony validator that looks for the attributes:

$validator = \Symfony\Component\Validator\Validation::createValidatorBuilder()
        ->enableAttributeMapping()
        ->getValidator();

Validate the object:

$embedding = new Embedding(
      'model1:embedding1',
      [0.1, 0.2, 0.3],
      ['source' => 'test']
    );

/** @var \Symfony\Component\Validator\ConstraintViolationListInterface $violations */
$violations = $validator->validate($embedding);
🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

nit: looks like some of the docblock indentations are incorrect. I assume this would be fixed automatically with phpcbf?

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

@kingdutch wrote a great blog post on how using Symfony Runtime could help solve some of these issues https://www.alexandervarwijk.com/blog/2025-09-08-proposal-restructuring-...

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

In this issue, I showed how you can create a value object and validate it using Symfony Validator. It's a pretty clean solution IMO. 📌 Create an 'embedding' object that can be validated Active

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

I like @danielveza 's idea of

I wonder if we can turn these into plugins or do something a little nicer than elseifs checking strings.

🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

We should decide on whether to fix the getConnector() fails or just duplicate some of the code and remove the trait.

Production build 0.71.5 2024