Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add API to get list of COS objects #1221

Merged
merged 1 commit into from
May 3, 2023
Merged

Add API to get list of COS objects #1221

merged 1 commit into from
May 3, 2023

Conversation

jaywcarman
Copy link
Member

@jaywcarman jaywcarman requested a review from bdunne as a code owner April 27, 2023 22:29
@jaywcarman jaywcarman mentioned this pull request Apr 27, 2023
Closed
@miq-bot miq-bot added the wip label Apr 27, 2023
@kbrock
Copy link
Member

kbrock commented Apr 28, 2023

ok

  • build is failing because it is expecting a feature for this
  • also, you added "supports => true", so I am expecting IbmCloud::...::CloudObjectStoreObject to have supports :create.
  • I didn't see the supports. Possibly you already added this in an unmerged PR, or very possibly I'm misremembering what :supports means in this context.

Thanks Jay for all the great work.

kbrock
kbrock reviewed May 2, 2023
View reviewed changes
config/api.yml Outdated Show resolved Hide resolved
kbrock
kbrock reviewed May 2, 2023
View reviewed changes
app/controllers/api/cloud_object_store_objects_controller.rb Outdated
class CloudObjectStoreObjectsController < BaseProviderController
def create_resource(type, _ems_id = nil, data = {})
create_ems_resource(type, data, :supports => true) do |ems, klass|
resource_search(data["cloud_tenant_id"], :cloud_tenants) if data["cloud_tenant_id"]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is cloud_tenant_id required?
If so, lets make that change accordingly.
If not, then this code is good as is

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Original scope of this PR is just read-only... so I just removed the create_resource method altogether.

@jaywcarman jaywcarman mentioned this pull request May 3, 2023
Open
2 tasks
@jaywcarman jaywcarman force-pushed the powervs_import_image_from_cos branch from d0d35f6 to 63cee2b Compare May 3, 2023 17:55
@jaywcarman
Copy link
Member Author

I added some changes to CloudTemplatesController earlier that in retrospect would be better off in a separate PR. Just removed it with a force push and opened #1223.

@jaywcarman jaywcarman force-pushed the powervs_import_image_from_cos branch from 63cee2b to 9af7d45 Compare May 3, 2023 18:44
@jaywcarman
Copy link
Member Author

  • build is failing because it is expecting a feature for this

Took me a while to realize it, but the feature was already there... I just typo'd the identifier 😆

diff --git a/config/api.yml b/config/api.yml
index f97de0fe..da5563ef 100644
--- a/config/api.yml
+++ b/config/api.yml                                   
@@ -610,7 +610,7 @@            
         :identifier: cloud_object_store_container_show
   :cloud_object_store_objects:            
     :description: Cloud Object Store Objects
-    :identifier: cloud_object_store_objects
+    :identifier: cloud_object_store_object
     :options:                     
     - :collection       
     :verbs: *gp

@jaywcarman jaywcarman changed the title [WIP] Add API to get list of COS objects Add API to get list of COS objects May 3, 2023
@jaywcarman jaywcarman changed the title Add API to get list of COS objects [WIP] Add API to get list of COS objects May 3, 2023
@jaywcarman
Copy link
Member Author

Existing specs are now passing locally... just need to add new specs and then this will be out of WIP.

@jaywcarman
Add support for /api/cloud_object_store_objects
1b6690c 
Add a new API to:
- Get cloud object store (COS) object
- Get a list of cloud object store (COS) objects
@jaywcarman jaywcarman force-pushed the powervs_import_image_from_cos branch from 9af7d45 to 1b6690c Compare May 3, 2023 19:45
@jaywcarman jaywcarman changed the title [WIP] Add API to get list of COS objects Add API to get list of COS objects May 3, 2023
@jaywcarman
Copy link
Member Author

jaywcarman commented May 3, 2023 •
edited
Loading

@kbrock I think this is ready to go now.

I wasn't sure if I should go ahead and add the :custom_actions: option (Similar to COS container here). I'm not familiar with this feature, but from reading the docs it looks like this is just generic support for custom buttons. For my purposes I don't need it, but on the other hand it could be nice to go ahead and enable it for someone down the road.

@miq-bot
Copy link
Member

miq-bot commented May 3, 2023

Checked commit jaywcarman@1b6690c with ruby 2.6.10, rubocop 1.28.2, haml-lint 0.35.0, and yamllint
2 files checked, 0 offenses detected
Everything looks fine. 👍

@miq-bot miq-bot removed the wip label May 3, 2023
@kbrock kbrock merged commit 36038d0 into ManageIQ:master May 3, 2023
@jaywcarman jaywcarman deleted the powervs_import_image_from_cos branch May 3, 2023 20:56
@Fryguy Fryguy added this to the Petrosian milestone Sep 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kbrock kbrock kbrock left review comments

@bdunne bdunne Awaiting requested review from bdunne bdunne is a code owner

Assignees

@kbrock kbrock

Labels
enhancement
Projects
Roadmap
Status: Petrosian
Milestone
Petrosian
Development

Successfully merging this pull request may close these issues.
4 participants
@jaywcarman @kbrock @miq-bot @Fryguy