Deprecate Yao::Resources::RestfullyAccessible.return_resources, Yao::Resources::*.list_detail

hiboma commented 5 years ago

Hi. This PR depcates Yao::Resources::RestfullyAccessible.return_resources same as https://github.com/yaocloud/yao/pull/126.

Yao::Resources::RestfullyAccessible.return_resources を廃止することを提案する PR です

Draft PR で相談したいこと

return_resources を消していく中で、特定のリソースで list_detail の実装がコピペ状態になっているのを見つけました

      # @param query [Hash]
      # @return [Array<Yao::Resources::*>]
      def list_detail(query={})
        res = GET([resources_path, "detail"].join("/"), query)
        resources_from_json(res.body)
      end

この list_detail の実装を Yao::Resources::* から Yao::Resources::RestfullyAccessible に集約するとでコードが DRY になるだろうと考えています

迷っていること

上記のような変更を入れると、 OpenStack API で /${resource_path}/details を持たないリソースにも list_detail が生えてしまうのでどのように集約するのがいいかなぁと思案しています

buty4649 commented 5 years ago

/${resource_path}/details を持っているリソースはそこまで多くないので、不要なリソースにはやしてしまうと混乱してしまいそうですね。ジャストアイディアですがlist_detail部分だけモジュールかして利用するリソースからincludeする形にするのはどうでしょうか

hiboma commented 5 years ago

第一の案

素朴に module に分離すると以下のような名前をきって list_detail を include する感じかな〜

Yao::Resources:::AccessibleResourcesDetail
Yao::Resources:::HasResourcesDetail
Yao::Resources:::RestfullyAccessibleResourceDetail (一貫性あるけど長い!)

hiboma commented 5 years ago

別の案

もしくは以下のように新しいクラス変数を付けておいて、全ての Yao::Resource::* で list_detail メソッドを生やしはするけど details を持たないリソースでは誤用を避けるようにガード節を入れるか、かなぁ

module Yao::Resources
  class Flavor < Base
    self.service        = "compute"
    self.resource_name  = "flavor"
    self.resources_name = "flavors"
    self.accessible_resource_details = true 👈 🆕 
    end
  end
end

module Yao::Resources
  module RestfullyAccessible

...

    def list_detail(query={})

      # 🔥 detail を持たないクラスでは実行できない
      unless accessible_resource_details
        raise '#{self.to_s} does not have resources detail API.'
      end

      res = GET([resources_path, "detail"].join("/"), query)
      resources_from_json(res.body)
    end

hiboma commented 5 years ago

第三の案

既存の https://github.com/yaocloud/yao/blob/master/lib/yao/resources/metadata_available.rb に揃えようかなぁ

ResourceDetailAvailable ?

buty4649 commented 5 years ago

別の案の内容を見て思ったのですが、そもそもlistとlist_detailを分ける必要があるのかなっと :thinking: listで取れるものはlist_detailでとれるので、accessible_resource_detais=trueなリソースは、listでdetailsを叩くようにすると良いのかなっと思いました。

hiboma commented 5 years ago

＼💡 ／

hiboma commented 5 years ago

Draft PR 解除までに諸々やったことです

Deprecate return_resources
Integrate list_detail to list
- Introduce a new feature self.resources_detail_available
- Rewrite tests for list_detail to list
- Add a test for Yao::Volume.list

粒度がだいぶ大きくなったけどよろしくお願いします @buty4649

hiboma commented 5 years ago

Yaoの互換性を保つために一時的に alias :list_detail :list するのはいかかでしょうか？そして、折をみてlist_detailを削除するみたいなのを考えました。

ok 〜

hiboma commented 5 years ago

Add alias :list_detail list and the tests したぜ!

hiboma commented 5 years ago

Appreciate your thoroughly review ! 細かいとこまでありがと〜

yaocloud / yao