CommentFileSizeAuthor
#21 hotfix-3158962-21.patch584 bytesyas
#20 hotfix-3158962-20.patch584 bytesyas
#15 hotfix-3158962-15.patch680 bytesyas
#9 interdiff-3158962-9.txt2.99 KByas
#7 3158962-7.patch14.95 KBXiaohua Guan
#6 3158962-6.patch14.94 KBXiaohua Guan
#5 dev1.dataninja.net_admin_reports_dblog.png2.05 MByas
#5 dev1.dataninja.net_admin_config_services_cloud_openstack_settings.png418.02 KByas
#2 3158962-2.patch14.8 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
FileSize
3158962-2.patch14.8 KB
Xiaohua Guan’s picture

Xiaohua Guan CreditAttribution: Xiaohua Guan commented
Status: Active » Needs review
Xiaohua Guan’s picture

Xiaohua Guan CreditAttribution: Xiaohua Guan commented

@yas

Please review the patch file. Thanks.

FYI, although I think the code should work, but I don't have openstack environment, so I can't confirm it in the REAL environment. Could you run cron job for the openstack enironment?

yas’s picture

yas
🇯🇵 he/him
Primary language Japanese
Location California 🇺🇸
CreditAttribution: yas at DOCOMO Innovations, Inc. commented
Title: Refactor openstack cron job » Refactor OpenStack cron job
FileSize
dev1.dataninja.net_admin_config_services_cloud_openstack_settings.png418.02 KB
dev1.dataninja.net_admin_reports_dblog.png2.05 MB

@xiaohua-guan

Thank you for the refactoring. I tested the patch and it looks good to me.

admin_config_services_cloud_openstack_settings.png
reports_dblog.png

One thing that I was concerned is the variable $ec2_service, $ec2_method_name and 'ec2_method_name'. Those prefix can be $openstack_*? $ec2_service can be refactored in all occurrences incl. the existing ones, but limited to the patch, I thought it would be better to put $optnstack_ec2_service. What do you think? (The type of the recent log messages can be also openstack_ec2_service if possible?)

Xiaohua Guan’s picture

Xiaohua Guan CreditAttribution: Xiaohua Guan commented
FileSize
3158962-6.patch14.94 KB
Xiaohua Guan’s picture

Xiaohua Guan CreditAttribution: Xiaohua Guan commented
FileSize
3158962-7.patch14.95 KB

@yas

I added a prefix openstack_ or openStack. Could you check it? Thanks.

The last submitted patch, 6: 3158962-6.patch, failed testing. View results

yas’s picture

yas
🇯🇵 he/him
Primary language Japanese
Location California 🇺🇸
CreditAttribution: yas at DOCOMO Innovations, Inc. commented
FileSize
interdiff-3158962-9.txt2.99 KB

@xiaohua-guan

Thank you for the update. It looks good to me now.

@baldwinlouie

What do you think?

baldwinlouie’s picture

baldwinlouie CreditAttribution: baldwinlouie commented
Status: Needs review » Reviewed & tested by the community

@yas and @xiaohua-guan,

The patch looks good to me!

yas’s picture

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

@baldwinlouie

Thank you for your review. I'll merge the patch to 8.x-1.x and 8.x-2.x and close this issue as Fixed.

  • yas committed 571ff69 on 8.x-1.x authored by Xiaohua Guan 
    Issue #3158962 by Xiaohua Guan, yas, baldwinlouie: Refactor OpenStack...

  • yas committed e958842 on 8.x-2.x authored by Xiaohua Guan 
    Issue #3158962 by Xiaohua Guan, yas, baldwinlouie: Refactor OpenStack...
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 review
FileSize
hotfix-3158962-15.patch680 bytes

@all Fixing an error of in openstack.install

  • yas committed cbd5ca7 on 8.x-1.x 
    Issue #3158962 by Xiaohua Guan, yas, baldwinlouie: Hotfix - Refactor...

  • yas committed 936be1e on 8.x-2.x 
    Issue #3158962 by Xiaohua Guan, yas, baldwinlouie: Hotfix - Refactor...
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

@all I merged the patch since all tests have been passed successfully.

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 review
FileSize
hotfix-3158962-20.patch584 bytes

@all Fixing the description of hook_update_N()

yas’s picture

yas
🇯🇵 he/him
Primary language Japanese
Location California 🇺🇸
CreditAttribution: yas at DOCOMO Innovations, Inc. commented
FileSize
hotfix-3158962-21.patch584 bytes

Trying again.

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

@all I will merge the patch since all tests have been passed successfully.

  • yas committed a22356a on 8.x-1.x 
    Issue #3158962 by yas, Xiaohua Guan, baldwinlouie: Refactor OpenStack...

  • yas committed 4f269da on 8.x-2.x 
    Issue #3158962 by yas, Xiaohua Guan, baldwinlouie: Refactor OpenStack...
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

Status: Fixed » Closed (fixed)

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