Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mospy sentinel chain #24

Merged
merged 2 commits into from
Jan 3, 2024
Merged

mospy sentinel chain #24

merged 2 commits into from
Jan 3, 2024

Conversation

Tkd-Alex
Copy link
Contributor

@Tkd-Alex Tkd-Alex commented Jan 2, 2024

Hello Felix, first of all thank for your awesome work! ❤️
We are developing a sentinel_sdk in python and I had to add the sentinel_protobuf inside mospy.

Currently the sentinel_protobuf is maintained by @MathNodes. I saw that 2 weeks ago you accept the PR ctrl-Felix/cosmospy-protobuf#14, but the pypi release is Nov 27, 2023 (without the sentinel_protobuf).

I had to change the requirements from cosmospy_protobuf to sentinel_protobuf.
Now we have two options:

  1. Continue to maintain separate the protobuf and mospy package, obviously will be create a pypi package sentinel_mospy
  2. Pubblish on pypi the latest version of cosmospy_protobuf (that include also sentinel) and the release an update also for mospy (this PR - with the requirements.txt rollbacked to cosmospy_protobuf)

What do you think?

The PR include also a GetTx method, out of sentinel/chain related scope.

@ctrl-Felix
Copy link
Owner

Thanks for your contribution :)

I've seen the sentinel protobufs and I am very happy to see that there's a community contributed protobuf package!

Regarding the integration into mospy. I think there's not specific need for a sentinel_mospy package, at least not on my end. There's no osmosis_mospy package either. And you can already use sentinel with the current mospy package. Because you can pass "sentinel_protobuf" when creating the Account/Transaction instance which will work. The dict with the protobuf names is just to add an alias. But I think it's a good idea to add sentinel to these too.

Regarding the get Tx. I want to improve the clients anyways but to keep them both consistent I'd prefer to make a new pr/branch with a proper client update.

@Tkd-Alex
Copy link
Contributor Author

Tkd-Alex commented Jan 3, 2024

as you request I've moved the GetTx in to another pull request (#25), I've rollbacked the requirements.txt

@ctrl-Felix ctrl-Felix merged commit 192c612 into ctrl-Felix:master Jan 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants