🇬🇧United Kingdom @vijaycs85

Here is the initial patch.

vijaycs85 → created an issue.

Working on CI failures

managed to create MRs directly from the gitlab merge request URL.

rerolled and fixed PHPCS issues on the MR at 3.x as well.

rerolled against latest 2.x

> Did you click "Get Push Access"?

I don't see the button (attached screenshot for reference).

> Also all changes go into 3.x first.
yeah, I have a branch for 3.x I will push it once we have some reviews in place.

thanks! I pushed changes to 2.x at https://git.drupalcode.org/issue/search_api_opensearch-3513714/-/tree/2.... I can't create MR from the fork as I might not have access.

vijaycs85 → created an issue.

Can you please confirm the versions of search_api_opensearch and opensearch-project/opensearch-php you are using?

drupal/search_api_opensearch:      2.x-dev 
opensearch-project/opensearch-php: 2.4.2

vijaycs85 → created an issue.

Added update hook.

1. Should we have a config migration path?

you mean add an update hook?

2. The wording on the form description isn't that helpful to those who don't know what the possible values are.

Now this is a dropdown, so helps what services are available.

3. Should we have an options list with just OpenSearch and OpenSearch Serverless?

yeah, makes sense. updated.

4. Can we just default to OpenSearch for existing users.

Ideally yes as the library sets default to es.

P.S: I am pushing changes to both MR and patch as I need to apply on my project in latest 2.x.

Thanks @kim.pepper. For our setup, we are just missing service, so doesn't make sense to have custom credential provider. I have attached the patch to introduce the service field in settings form.

+1. I can confirm there are two issues:
1. Reading credentials from AWS_CONTAINER_CREDENTIALS_RELATIVE_URI environment variable and get the key, secret, token & expire.
2. Service setup.

For the first issue, if we don't pass the details via the form or settings.php, it would use CredentialProvider::defaultProvider() and reads & fills the details. However for the service setup, there is no option at the moment. Here is the simple working script on ECS:

<?php

use Aws\Credentials\CredentialProvider;
use Drupal\Core\Utility\Error;
use OpenSearch\ClientBuilder;

$config = [
  'url' => 'https://[server-id].[region].aoss.amazonaws.com',
  'ssl_verification' => TRUE,
  'region' => '[region]',
  'service' => 'aoss',
];
$provider = CredentialProvider::defaultProvider();

$client = ClientBuilder::create()
  ->setHosts([$config['url']])
  ->includePortInHostHeader(TRUE)
  // To see the connection issues. can be removed once done with debugging the issues.
  ->setConnectionParams([
    'client' => [
      'curl' => [
        \CURLOPT_VERBOSE => 1,
      ]
    ]
  ])
  ->setSSLVerification($config['ssl_verification'])
  ->setSigV4Region($config['region'])
  ->setSigV4Service($config['service'])
  ->setSigV4CredentialProvider($provider)
  ->build();

try {
 $response = $client->indices()->create(['index' => 'projects-index-1']);
  print_r($response);
  $data = [
    'title'=> 'Drupal',
    'namespace' => 'drupal/drupal',
    'version' => '10.4.3'
  ];

  $response = $client->create([
    'index' => 'projects-index-1',
    'body' => json_encode($data),
    'id' => 't' . time(),
  ]);
  print_r($response);

}
catch (\Exception $e) {
  $error = Error::decodeException($e);
  print_r($error['@message']);
  print_r($error['@backtrace_string']);
}

scott_euser → credited vijaycs85 → .

Changes look good to me.

Closing this issue as the changes in patch aren't really Drupal coding standard fixes. Feel free to open new issue with PHPCS run with Drupal standard.

Thanks for the offer @dileepkumargembali The module does not have a lots of activity (bugs or features) and has been stable since introduced in Drupal 8. In fact most of the releases we made were purely the core version compatibility. Since we already have 3 maintainers, we don't need additional maintainers here.

+++ b/epsilon_harmony.info.yml
@@ -1,6 +1,6 @@
+core_version_requirement: '>=10'

This is a breaking change for any site using this module in Drupal 8/9. It also means we are allowing any future break. Since there is no 8/9 breaking changes introduced, I would still keep the versions.

The changes in https://git.drupalcode.org/project/epsilon_harmony/-/merge_requests/1 makes more sense here. so I am merging it. will create a new release.

@dileepkumargembali lets create a separate issue for the gitlab file.

Here is the re-roll of the MR against 1.8.

Facing this issue on migrating link.

+++ b/src/Plugin/migrate/source/d8/ContentEntity.php
@@ -149,6 +149,13 @@ class ContentEntity extends SqlBase {
+          $unserialized_value = @unserialize($value);

As mentioned before worth having a flag instead of blind unserilize() on all fields.

rerolled without the changes in .gitlab-ci.yml as it is already in HEAD.

Facing this issue. I assume the fix is available from version >=3.31.

re-rolling the patch as it doesn't apply cleanly on gitlab-ci.yml file and watchdog entry as it is causing circular dependency.

Addressed the logger issue.

As mentioned in #25 and #33, the logger causing circular dependency (esp. with drush). I have added review comments in relavent lines in the MR #13

changes look good. thanks!

changes look good, but the test failure seems legit as the error message changed?

vijaycs85 → made their first commit to this issue’s fork.

Thank you.

Thank you! pushed to 1.x

Fix typo

yay! finally green. pushed to 2.0.x

Hope this would fix the test failures.

thanks @suzymasri.

Perhaps the solution moving forward is to create a 2.x (Webform 6.2.x) branch that supports D10, while keeping D9 support on 1.x (Webform 5.x)

yeah, though we are on beta on 1.x, it doesn't make sense to introduce breaking changes. I have created 2.0.x from current HEAD.

Thanks for working on this. Here is some review comments.

1. +++ b/composer.json
   @@ -35,6 +35,11 @@
   +        {
   +            "name": "Piyush Kumar (piyushsms91)",
   +            "homepage": "https://www.drupal.org/u/piyushsms91",
   +            "role": "Co-maintainer"
   

   We can't add the person here unless they are co-maintainer on drupal.org.

2. +++ b/tests/src/Functional/WebformEncryptEditSumissionsTest.php
   @@ -62,7 +67,7 @@ class WebformEncryptEditSumissionsTest extends BrowserTestBase {
   -    $this->drupalPostForm('/webform/test_encryption', $edit, 'Submit');
   +    $this->submitForm($edit, 'Save');
   
   +++ b/tests/src/Functional/WebformEncryptFunctionalTest.php
   @@ -102,7 +107,7 @@ class WebformEncryptFunctionalTest extends BrowserTestBase {
   -    $this->drupalPostForm('/webform/test_encryption', $edit, 'Submit');
   +    $this->submitForm($edit, 'Save');
   
   +++ b/tests/src/Functional/WebformEncryptUninstallTest.php
   @@ -53,18 +58,18 @@ class WebformEncryptUninstallTest extends BrowserTestBase {
   -    $this->drupalPostForm('/webform/test_encryption', $edit, 'Submit');
   +    $this->submitForm($edit, 'Save');
   
   @@ -124,7 +129,7 @@ class WebformEncryptUninstallTest extends BrowserTestBase {
   -    $this->drupalPostForm('/webform/test_encryption', $edit, 'Submit');
   +    $this->submitForm($edit, 'Save');
   

   wrong submit button value on update.

Most of the changes here are cleanup than necessary for upgrade. But they are good cleanup to go with, so leaving them in.

We need to include coverage for adding a new entity type from the form and make sure it appears in the views drop down. I have a feeling we need to clear the views cache on the submit of this form so that the hook_views_data would get regenerated, but test would show as if that's the case.

the config form looks good. probably we just need to add some test coverage.

Thanks @pwolanin. tested and confirmed it is working with custom headers.

Tested agains 2.0.0-beta2 (esp. for the new feature ✨ Support adding custom JWT headers and also make the headers available after decoding Fixed ) and looks good.

alexpott → credited vijaycs85 → .

I think the problem is with the media dependency on info.yml

@arthur.islamov could you provide interdiff → ?

Re-rolling against 8.9.x (and works for 8.8.x as well).

Manually tested the patch (8.7.x HEAD) with default_content module and the exported node has the layout configuration and node-level override details (as below).

"layout_builder__layout": [
        {
            "section": {
                "layout_id": "layout_onecol",
                "layout_settings": [],
                "components": {
                    "b9d1b078-51c2-4918-a69b-5a4b58e3a947": {
                        "uuid": "b9d1b078-51c2-4918-a69b-5a4b58e3a947",
                        "region": "content",
                        "configuration": {
                            "id": "block_content:18853fa3-6af3-44a2-a14e-799af4e46910",
                            "label": "Working block",
                            "provider": "block_content",
                            "label_display": 0,
                            "status": true,
                            "info": "",
                            "view_mode": "full",
                            "context_mapping": []
                        },
                        "additional": [],
                        "weight": 1
                    }
                },
                "third_party_settings": []
            }
        }
]