CommentFileSizeAuthor
#7 3163202-7.patch2.57 KBXiaohua Guan
#3 3163202-3.patch1.9 KBXiaohua Guan
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Xiaohua Guan created an issue. See original summary.

Xiaohua Guan’s picture

Xiaohua Guan CreditAttribution: Xiaohua Guan commented
Issue summary: View changes
Xiaohua Guan’s picture

Xiaohua Guan CreditAttribution: Xiaohua Guan commented
FileSize
3163202-3.patch1.9 KB
Xiaohua Guan’s picture

Xiaohua Guan CreditAttribution: Xiaohua Guan commented
Status: Active » Needs review

@yas

Please review the patch file. Thanks.

yas’s picture

yas
🇯🇵 he/him
Primary language Japanese
Location California 🇺🇸
CreditAttribution: yas at DOCOMO Innovations, Inc. commented

@xiaohua-guan

Thank you for fixing the issue.

+++ b/modules/cloud_service_providers/k8s/src/Plugin/Block/K8sBaseBlock.php
@@ -359,4 +359,29 @@ abstract class K8sBaseBlock extends BlockBase implements ContainerFactoryPluginI
+  protected function isValidCloudContext($cloud_context) {

Since this method can be used more commonly, Should we move this method to CloudConfigPluginManager::isValidCloudContext($cloud_context, $bundle) or CloudConfig::isValidCloudContext($cloud_context, $bundle)?

Xiaohua Guan’s picture

Xiaohua Guan CreditAttribution: Xiaohua Guan commented

@yas

Thanks for your comment.

I found the function in CloudConfig. How about to make some change and reuse it?

public static function checkCloudContext($cloud_context) {
Xiaohua Guan’s picture

Xiaohua Guan CreditAttribution: Xiaohua Guan commented
FileSize
3163202-7.patch2.57 KB
Xiaohua Guan’s picture

Xiaohua Guan CreditAttribution: Xiaohua Guan commented

@yas

Please review the patch file. Thanks.

yas’s picture

yas
🇯🇵 he/him
Primary language Japanese
Location California 🇺🇸
CreditAttribution: yas at DOCOMO Innovations, Inc. commented

@xiaohua-guan

Thank you for the update. 

+++ b/src/Entity/CloudConfig.php
@@ -294,21 +294,34 @@ class CloudConfig extends CloudContentEntityBase implements CloudConfigInterface
+    if (empty($bundle)) {
+      return TRUE;
+    }

It looks good but let me confirm if it is okay with always TRUE at the following caller code?

https://git.drupalcode.org/project/cloud/-/blob/3.x/src/Form/CloudConfig...

Xiaohua Guan’s picture

Xiaohua Guan CreditAttribution: Xiaohua Guan commented

@yas

I've confirmed the code. It should be OK because the "exists" function is used as below.

https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Render%21...

    // Verify that the machine name is unique.
    if ($element['#default_value'] !== $element['#value']) {
      $function = $element['#machine_name']['exists'];
      if (call_user_func($function, $element['#value'], $element, $form_state)) {
        $form_state
          ->setError($element, t('The machine-readable name is already in use. It must be unique.'));
      }
    }
yas’s picture

yas
🇯🇵 he/him
Primary language Japanese
Location California 🇺🇸
CreditAttribution: yas at DOCOMO Innovations, Inc. commented
Status: Needs review » Reviewed & tested by the community

@xiaohua-guan

Thank you for updating the patch and the information. I'll merge the patch to 8.x-2.x and 3.x and close this issue as Fixed.

  • yas committed 7c5e334 on 8.x-2.x authored by Xiaohua Guan 
    Issue #3163202 by Xiaohua Guan, yas: Fix the error of K8s Node Allocated...

  • yas committed 4e36389 on 3.x authored by Xiaohua Guan 
    Issue #3163202 by Xiaohua Guan, yas: Fix the error of K8s Node Allocated...
yas’s picture

yas
🇯🇵 he/him
Primary language Japanese
Location California 🇺🇸
CreditAttribution: yas at DOCOMO Innovations, Inc. commented
Status: Reviewed & tested by the community » Fixed
yas’s picture

yas
🇯🇵 he/him
Primary language Japanese
Location California 🇺🇸
CreditAttribution: yas at DOCOMO Innovations, Inc. commented
Status: Fixed » Needs work

I'll merge to 8.x-1.x, too for the patch at #3163932

  • yas committed 7d5b8e4 on 8.x-1.x authored by Xiaohua Guan 
    Issue #3163202 by Xiaohua Guan, yas: Fix the error of K8s Node Allocated...
yas’s picture

yas
🇯🇵 he/him
Primary language Japanese
Location California 🇺🇸
CreditAttribution: yas at DOCOMO Innovations, Inc. commented
Status: Needs work » Fixed

Closing this issue marked as Fixed.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.