search

liftbridge-io / go-liftbridge

Go client for Liftbridge. https://github.com/liftbridge-io/liftbridge
Apache License 2.0
68 stars 18 forks source link

Metadata: add a function to access the stream map #71

Closed Jmgr closed 4 years ago

Jmgr commented 4 years ago

This PR adds a function to access the stream map when fetching metadata. The current API only allows you to get a SteamInfo using a stream name.

tylertreat commented 4 years ago

I think we ought to return a copy of the map rather than the actual underlying one? Just to prevent the ability to manipulate it from outside of Metadata.

Jmgr commented 4 years ago

Hm yes that would be more coherent with the other functions. I'll make that change.

Shouldn't StreamInfo.Partitions() also return a copy in that case?

Jmgr commented 4 years ago

I have made the change in aee6d4c04f532b4371dee5cf0135986f47852a9e.

I have also committed a code simplification in 8f9c1e088be7aba01f71f3666305940c3621731d. Performance should be the same, so it's only a cosmetic change. What do you think?

Jmgr commented 4 years ago

Also, shouldn't those changes be made in the v2 module?

tylertreat commented 4 years ago

Apologies for the delay on the response, was out all week.

Yeah, we should include the changes in the v2 module.

Jmgr commented 4 years ago

Yeah, we should include the changes in the v2 module.

I have applied the changes to v2 metadata.go file and removed new function from v1.

tylertreat commented 4 years ago

I have applied the changes to v2 metadata.go file and removed new function from v1.

Since the change is backwards compatible, it's OK to include in v1 if you are reliant upon v1 of the module. I think larger API changes will only be applied to v2 going forward but I think it's fine to backport smaller changes to v1.

Jmgr commented 4 years ago

Since the change is backwards compatible, it's OK to include in v1 if you are reliant upon v1 of the module. I think larger API changes will only be applied to v2 going forward but I think it's fine to backport smaller changes to v1.

Alright. I have reverted the function removal from v1.