Looks like just a few PHPCS errors. Also tagging for Needs Tests. I think it would be good for a test to ensure it all get's wired up correctly.
Released in 2.4.0 →
Committed to 3.x and 2.x. Thanks!
Changes go into 3.x.
Updated title
Marking RTBC. Hoping other maintainers can review as well.
I agree this would be useful. OpenSearch will create an index when the first document is pushed, so this is existing behaviour. We are just making sure our mappings are correct when the index is created.
fubarhouse → credited kim.pepper → .
Currently we default to application/octet-stream if we can't determine the mime type. See https://developer.mozilla.org/en-US/docs/Web/HTTP/Guides/MIME_types#appl...
Any kind of binary data that doesn't fall explicitly into one of the other types;
See also 🐛 Separate MIME type mapping from ExtensionMimeTypeGuesser Needs work Not sure about the automatic extension change tho.
Yeah, not sure of the history of this one. I think we could do away with the check for apache and nginx and just display the default "extension missing" message.
Looks good. Can we add a test?
I'd say this is outdated since we just check if the extension is enabled now.
> I can't create MR from the fork as I might not have access.
Did you click "Get Push Access"?
Also all changes go into 3.x first.
I'm trying an alternative approach in https://git.drupalcode.org/project/drupal/-/merge_requests/11533 however I can't tell if test fails are related or not? 🤔
Instead of adding onto the Time class, I'm creating a dedicated Clock implementation.
All good now.
Reviewed this and checked the property can still be accessed and there is a test to trigger the deprecation.
RTBC once we have a CR.
> I imagine this will need a CR?
Yep.
> How would we want to handle the removal of this code in D12? Do we wait for this to be committed and open a follow up?
I think this would get handled when we make deprecation removal issues when 12.x opens up.
I'm seeing a few changes that look unrelated. You might need to rebase on 11.x?
Yeah I think a sub-classed search backend would work. I think a sub-module in this project would make more sense since there would be minimal differences.
Left one nitpick.
kim.pepper → changed the visibility of the branch 3501375-symfony-clock to hidden.
I created a follow-up 📌 [PP-1] Deprecate TimeInterface::getCurrentTime() and replace with ClockInterface::now()->getTimestamp() Postponed so back to RTBC.
I also squashed the commits and rebased on the latest 11.x. This will help if we need to continually rebase on 11.x until it gets committed.
kim.pepper → created an issue.
We might want to do #17 and deprecate \Drupal::time()->getCurrentTime()
in a follow up.
I was creating a follow up issue to replace TimeInterface->getCurrentTime()
with ClockInterface->now()->getTimestamp()
in the rest of core.
I searched for usages of \Drupal::time()->getCurrentTime()
and found 28 instances.
\Drupal::time()
returns \Drupal\Component\Datetime\TimeInterface
but this doesn't extend Psr\ClockInterface
, so it's not a straight method swap.
I think we need a new \Drupal::clock()
method that returns Psr\ClockInterface
. This would make it easier to swap out usage of \Drupal::time()->getCurrentTime()
with \Drupal::clock()->now()->getTimestamp()
.
I know there is an existing issue about this somewhere but I can't find it.
Reviewed and added some feedback.
Added a CR and updated the IS. I think this is ready.
I think this was fixed in 📌 Use \OpenSearch\Aws\SigningClientFactory Active with this code:
// Set credentials if provided, otherwise fall back to defaults.
if ('' !== $this->configuration['api_key'] && '' !== $this->configuration['api_secret']) {
$options['auth_aws']['credentials'] = [
'access_key' => $this->configuration['api_key'],
'secret_key' => $this->configuration['api_secret'],
];
}
Because this uses logic in the library, we can only check to see if the 'credentials' array key gets set. See https://github.com/opensearch-project/opensearch-php/blob/main/src/OpenS...
// Check for provided access key and secret.
if (isset($options['credentials'])) {
return CredentialProvider::fromCredentials(
new Credentials(
$options['credentials']['access_key'] ?? '',
$options['credentials']['secret_key'] ?? '',
$options['credentials']['session_token'] ?? null,
)
);
}
// Fallback to the default provider.
return CredentialProvider::defaultProvider();
Are you able to manually test this now? If there are issues with the opensearch-php library, feel free to create an issue there too.
📌 Use \OpenSearch\Aws\SigningClientFactory Active is in so this is unblocked.
Committed to 3.x and 2.x.
opensearch-php 2.4.3 has been released
We looked at this at the first-time contributor workshop at DrupalSouth 2025. The following users helped test this manually but could not reproduce:
atowl, lan, yi_jiang, Ronbot, nterbogt, silverham, tarawij, ben_a, cordo1, jimmycann, tyrellblackburn,
We tested by:
1. Deleting the version from the standard.info.yml file (which we installed)
2. Clearing our cache
3. Visiting the Status Report page
Can you please provide more steps to reproduce.
kim.pepper → made their first commit to this issue’s fork.
If we are adding Psr/Clock as a dependency, then there should be at least one instance where we swap out to use it.
I'm wondering if we don't just add symfony/clock and replace usages of datetime.time service
? 🤔
@vijaycs85 would be great if you could test out 📌 Use \OpenSearch\Aws\SigningClientFactory Active and see if it fixes your issues before we commit it.
We should postpone this on a tagged release of the opensearch-php
lib.
I think this is due to not checking for empty strings when passing the credentials options.
I'm going to postpone this one on 📌 Use \OpenSearch\Aws\SigningClientFactory Active which changes how we create the signing client.
kim.pepper → created an issue.
\Drupal\Core\File\MimeType\MimeTypeGuesser::guessMimeType()
defaults to 'application/octet-stream'
if it can't determine a mime type. Would it make sense to default to this rather than empty string?
I think this is the expected behaviour.
- '_' is a valid separation character and santization only replaces invalid characters.
- '_-' gets converted to '-' because we de-duplicate separation characters
Reviewed and confirmed the links are correct.
Thanks for reporting @vijaycs85
Can you please confirm the versions of search_api_opensearch
and opensearch-project/opensearch-php
you are using?
NW for the code style fails
Committed to 3.x and 2.x
kim.pepper → created an issue.
I ran the test-only pipeline and it fails, but not where I expected it to. Any thoughts?
1) Drupal\KernelTests\Core\File\StreamWrapperCustomRootDirectoryTest::testRealPathHandlesCustomRootDirectory
TypeError: stat(): Argument #1 ($filename) must be of type string, false given
The temporary file path is a setting
and no longer config since Drupal 8.8.0.
See the changelog: https://www.drupal.org/node/3039255 → for details.
Can we update the IS to mention this only affects root directories (or symlinks to them)?
1. Should we have a config migration path?
you mean add an update hook?
Yep.
Would be great to have a test that covered this.
RTBC +1
I've rebased this on 11.x. In my view this is ready.
@smustgrave it's not as simple as moving the fields around. We need to merge the output from the parent \Drupal\file\Plugin\Field\FieldFormatter\FileMediaFormatterBase::settingsForm()
. Trying to splice it into the middle is going to make it pretty fragile imo.
I think we need to handle existing sites a bit better.
- Should we have a config migration path?
- The wording on the form description isn't that helpful to those who don't know what the possible values are.
- Should we have an options list with just OpenSearch and OpenSearch Serverless?
- Can we just default to OpenSearch for existing users.
kim.pepper → changed the visibility of the branch 3479880-allow-aws-signature to hidden.
Created a MR from the patch at #7
Sounds like a good idea.
Committed to 2.x. Thanks!
kim.pepper → made their first commit to this issue’s fork.
Committed to 3.x and 2.x. Thanks!
kim.pepper → made their first commit to this issue’s fork.
+++ b/modules/search_api_aws_signature_connector/src/Plugin/OpenSearch/Connector/AwsSignatureConnector.php
@@ -91,6 +93,17 @@ class AwsSignatureConnector extends StandardConnector {
+ ];
Should we default this to 'es'?
Fixed in 📌 Automated Drupal 10 compatibility fixes Fixed
Since we updated the opensearch-php library to 2.4.x you can use the \Drupal\search_api_aws_signature_connector\AwsSigningHttpClientFactory
and pass your own Aws\Credentials\CredentialProvider
in the service definition.
We might be able to make it an option in the settings form.
Thanks @webflo Can you please create an MR against 3.x as that's the active development branch. We can backport to 2.x.
The event is for altering validation constraints. If you are creating a new one, you probably want to implement your own ConstraintValidator
plug-in that extends \Drupal\file\Plugin\Validation\Constraint\BaseFileConstraintValidator
.
Committed to 8.x-1.x. Thanks!
Committed to 8.x-1.x
kim.pepper → created an issue.
Committed to 3.x and 2.x
kim.pepper → created an issue.
Commited to 2.x
Committed to 2.x
kim.pepper → created an issue.
Committed to 2.x
kim.pepper → created an issue.
Committed to 2x. and 3.x. Thanks!
Made it configurable and added an update hook. @yobottehg it would be great if you could test this out manually.
@yobottehg I've rebased this on 3.x.
Can you explain why you chose a value of 20 rather than any other arbitrary number? Also, should this be configurable?
New commits go into 3.x
Committed to 3.x.
Need to add the composer dependency.