Problem/Motivation

  • Add REST API to edit the OpenStack launch template in the Cloud module.

Issue fork cloud-3317494

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

Ryo Yamashita created an issue. See original summary.

Ryo Yamashita opened merge request !1384

yas’s picture

yas
🇯🇵 he/him
Primary language Japanese
Location California 🇺🇸
commented
Title: Add REST API to edit the Openstack launch template in the Cloud module » Add REST API to edit the OpenStack launch template in the Cloud module
Issue summary: View changes
Ryo Yamashita’s picture

Ryo Yamashita commented
Status: Needs work » Needs review

@yas

Please review it. Thanks.

yas’s picture

yas
🇯🇵 he/him
Primary language Japanese
Location California 🇺🇸
commented
Status: Needs review » Needs work

@ryo-yamashita

Thank you for the patch. I posted my comment. Thanks!

Ryo Yamashita’s picture

Ryo Yamashita commented
Status: Needs work » Needs review

@yas

I fixed and pushed the code. Thanks.

yas’s picture

yas
🇯🇵 he/him
Primary language Japanese
Location California 🇺🇸
commented
Status: Needs review » Needs work

@ryo-yamashita

Can you please check the following coding standard violation?

FILE: ...e_providers/openstack/src/Service/OpenStackOperationsService.php
----------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
----------------------------------------------------------------------
 2843 | WARNING | \Drupal calls should be avoided in classes, use
      |         | dependency injection instead
----------------------------------------------------------------------

Thanks

Ryo Yamashita’s picture

Ryo Yamashita commented

@yas

I have confirmed that warning.
But I couldn't figure out how to replace the part that reads out the Drupal::routeMatch() with DI.
If you know, I would appreciate it if you could let me know. Thanks.

Ryo Yamashita’s picture

Ryo Yamashita commented
Status: Needs work » Needs review
yas’s picture

yas
🇯🇵 he/him
Primary language Japanese
Location California 🇺🇸
commented
Status: Needs review » Needs work

@ryo-yamashita

  1. Add use Drupal\Core\Routing\RouteMatchInterface;.
  2. Add $routeMatch member variable.
  3. Add a parameter $route_match into the constructor.
  4. Add '@current_route_match' parameter into openstack.services.yml. (By the way, I think we should remove aws_cloud.openstack.operations from openstack.services.yml...?)
  5. Add $container->get('current_route_match') into the create() method.
  6. Use $this->routeMatch->getParameter('cloud_context');.

HTH

Ryo Yamashita’s picture

Ryo Yamashita commented

@yas

Thank you for your guidance. I will implement it.

Ryo Yamashita’s picture

Ryo Yamashita commented
Status: Needs work » Needs review

@yas

I pushed the patch for fixing DI. Thanks.

yas’s picture

yas
🇯🇵 he/him
Primary language Japanese
Location California 🇺🇸
commented
Status: Needs review » Reviewed & tested by the community

@ryo-yamashita

Thank you for the update. It looks good to me now. I'll merge the patch to 4.x and 5.x, and close this issue as Fixed.

yas’s picture

yas
🇯🇵 he/him
Primary language Japanese
Location California 🇺🇸
commented
Status: Reviewed & tested by the community » Needs work

@ryo-yamashita

Can you please rebase the patch? Thanks!

Ryo Yamashita’s picture

Ryo Yamashita commented
Status: Needs work » Needs review
yas’s picture

yas
🇯🇵 he/him
Primary language Japanese
Location California 🇺🇸
commented
Status: Needs review » Reviewed & tested by the community

@ryo-yamashita

Thank you for rebasing the patch. I'll merge the patch to 4.x and 5.x, and close this issue as Fixed.

  • yas committed d2328df on 5.x authored by Ryo Yamashita 
    Issue #3317494 by Ryo Yamashita, yas: Add REST API to edit the OpenStack...

  • yas committed 0503614 on 4.x 
    Issue #3317494 by Ryo Yamashita, yas: Add REST API to edit the OpenStack...
yas’s picture

yas
🇯🇵 he/him
Primary language Japanese
Location California 🇺🇸
commented
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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