fabric-admin-sdk icon indicating copy to clipboard operation
fabric-admin-sdk copied to clipboard

moved osn package

Open adityajoshi12 opened this issue 1 year ago • 7 comments

adityajoshi12 avatar Apr 27 '24 09:04 adityajoshi12

As a personal suggestion, if @denyeart/ @bestbeforetoday can offline review this PR that would be fine. otherwise, @aldousalvarez , you'd better join fabric contributor meeting or other approach to contact with Dave or Mark.

SamYuan1990 avatar Apr 27 '24 11:04 SamYuan1990

Hi @denyeart , can you review this PR?

adityajoshi12 avatar May 14 '24 09:05 adityajoshi12

I'm sure there was a discussion somewhere else but I don't find it linked to this PR.

The osnadmin package that this PR exposes in the public API is pretty low-level. I'm not sure if *http.Response is really the ideal return type for users of fabric-admin-sdk. Much of the newly exposed functionality also duplicates (slightly) higher level capability already exposed by the channel package.

What exactly are we trying to achieve with this PR?

bestbeforetoday avatar May 14 '24 10:05 bestbeforetoday

I'm sure there was a discussion somewhere else but I don't find it linked to this PR.

The osnadmin package that this PR exposes in the public API is pretty low-level. I'm not sure if *http.Response is really the ideal return type for users of fabric-admin-sdk. Much of the newly exposed functionality also duplicates (slightly) higher level capability already exposed by the channel package.

What exactly are we trying to achieve with this PR?

Hi @denyeart Currently, fabric-admin-sdk doesn't exposes the public API to deal with channel operations related to orderer, like join, remove, etc. The changeset in this PR make the osn package public so that it can be consumed in downstream application, similar to hlf-connector which uses fabric-sdk-java to deal with such operations

adityajoshi12 avatar May 14 '24 10:05 adityajoshi12

I see a channel.CreateChannel function, which does the same thing as osnadmin.Join. There is also a channel.ListChannel function, which does the same thing as osnadmin.ListAllChannels. The functions in the channel package seem to offer a (slightly) more friendly API for application code, doing some marshalling of input parameters and unpacking HTTP responses. Perhaps we just could refine and extend the approach of the channel package instead of expose the low-level osnadmin package?

bestbeforetoday avatar May 14 '24 11:05 bestbeforetoday

I see a channel.CreateChannel function, which does the same thing as osnadmin.Join. There is also a channel.ListChannel function, which does the same thing as osnadmin.ListAllChannels. The functions in the channel package seem to offer a (slightly) more friendly API for application code, doing some marshalling of input parameters and unpacking HTTP responses. Perhaps we just could refine and extend the approach of the channel package instead of expose the low-level osnadmin package?

Thanks for the reference, however it still misses few functionality like, orderer removal, also would be nice if we have high level API that adds the orderer to the consenter list.

adityajoshi12 avatar May 14 '24 14:05 adityajoshi12

Can we extend the channel package with the required capability, exposed in a more application-friendly manner than osnadmin? Not just turn osnadmin into public API.

It's a good opportunity to see if what's there can be improved too. At first glance, having *http.Response as a return value doesn't look ideal. Maybe some different naming would improve clarity too.

bestbeforetoday avatar May 14 '24 15:05 bestbeforetoday