juju-solutions / libcharmstore

Other
1 stars 6 forks source link

Remove more charmstore query data #9

Closed ChrisMacNaughton closed 5 years ago

ChrisMacNaughton commented 6 years ago

Additionally, makes two of the fields on-demand only, as libcharmstore nor amulet actually reference them. The API remains unchanged but the amount of data requested from the charmstore should be dramatically reduced.

gnuoy commented 6 years ago

LGTM. As you said @ChrisMacNaughton there is a risk of someone accessing self.raw directly but I think its a risk worth taking.

johnsca commented 6 years ago

Oh, I also meant to ask... This wraps theblues, but why is the wrapper necessary rather than just using theblues directly? Is it just to provide a nicer API? I was also under the impression that theblues was basically unmaintained; if this is maintained and used, would it make more sense to fold the actual request logic into this as well? If nothing else, the name is way better.

ChrisMacNaughton commented 6 years ago

I'm very much +1 to updating libcharmstore to handle the charmstore API rather than using theblues

marcoceppi commented 6 years ago

@johnsca At the time this library was created theblues didn't really have a good public interface. If theblues is unmaintained then we may want to consider folding request logic into this as well. We're still woefully young in the library's release cycle (0.0) so planning a 1.0 with request logic folded in and improvements to some of the less mature parts of the API would be great.