search

xlab-steampunk / redfish-client-python

Minimalistic Redfish API client
Apache License 2.0
4 stars 8 forks source link

Lazy-load resources #19

Closed mancabizjak closed 3 years ago

mancabizjak commented 3 years ago

Until now, we eagerly loaded Resource's content as soon as we built it. This proved to be a bit awkward for resources with many siblings, as accessing the contents of one such resource resulted in fetching of the sibling resources' contents as well.

With this commit, we substitute this behavior with lazy loading. Rather than fetching the content of a resource immediately upon being built, we delay it until it is accessed.

mancabizjak commented 3 years ago

Thank you for the comments @matejart. I think I addressed all of them, and I prefer the revised version over the original one.

The most significant change is allowing for either eager or lazy resources rather than substituting one with the other entirely. The default is still eager loading, but the client will now create lazy resources in case lazy_load=True is provided to redfish_client.connect.

tadeboro commented 3 years ago

The most significant change is allowing for either eager or lazy resources rather than substituting one with the other entirely. The default is still eager loading, but the client will now create lazy resources in case lazy_load=True is provided to redfish_client.connect.

I know no one asked for my opinion here, but do we have a use case where lazy loading would break things? I am asking this because the only reason things were not lazy from the beginning is my laziness when I initially rewrote the Ruby Redfish client into Python. In Ruby client, fetching data eagerly made sense because we used it primarily to populate inventory. We actually needed all of the data retrieved in that use case. But even in that use case, laziness would not break anything (and would definitely alleviate some problems we had with hitting BMCs too hard with a series of requests).

If we do not have a valid use case for eager fetch right now, I would say YAGNI wins, and we should hardcode laziness into the client. But then again, @mancabizjak already wrote some seriously tested implementation, so feel free to ignore my comments.

matejart commented 3 years ago

Thank you, @tadeboro, your opinion is welcome and spot-on. The reason I suggested the compatibility mode was, well, I didn't think it through quite thoroughly enough and was worried about a change, which should actually be transparent.

Given the work and all the tests, I suggest we flip the default to always lazy.