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

Allow passing extra GRPC services to AdminServer #3242

Merged
merged 2 commits into from
Mar 16, 2023

Conversation

eranrund
Copy link
Contributor

Motivation

A bunch of our applications already include this internal, privileged GRPC server, and it could be convenient to use it for extra app-specific functionality.

Specifically, I want to add some admin commands to the DEQS liquidity bot, and it already has this GRPC server, so it seemed like the right place for it, but at the moment there's no way to add extra GRPC services so I either copy-paste AdminServer, or we extend it to allow passing of extra GRPC services.

A bunch of our applications already include this internal, privileged GRPC server, and it could be convenient to use it for extra app-specific functionality.
@cbeck88
Copy link
Contributor

cbeck88 commented Mar 15, 2023

If you want this for deqs repo, are you going to bump deqs repo to point at master?
I thought we have it on 4.1 for compatibility

@eranrund
Copy link
Contributor Author

If you want this for deqs repo, are you going to bump deqs repo to point at master? I thought we have it on 4.1 for compatibility

Oops yeah this should be against release/v4.1, thanks

@eranrund eranrund changed the base branch from master to release/v4.1 March 15, 2023 02:36
Copy link
Contributor

@cbeck88 cbeck88 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems fine to me

@jcape
Copy link
Contributor

jcape commented Mar 15, 2023

This should be pointed at master: we haven't done any testing on this at all, it wasn't brought up in any meeting, and we're releasing in < 6 hours.

5.0 is (tentatively) scheduled for release on Monday, there's literally zero point to put this into the stable branch at this late a stage.

Copy link
Contributor

@jcape jcape left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's too late to add new features to 4.1.

@cbeck88
Copy link
Contributor

cbeck88 commented Mar 15, 2023

Maybe we can do a 4.1+dev branch for the deqs so that we arent building against master for prod?

@eranrund
Copy link
Contributor Author

I don't remember whats getting into 5.0 but maybe we can just move the DEQS to point at 5?

@jcape
Copy link
Contributor

jcape commented Mar 15, 2023

The current plan is for master to become release/v5.0 next week, and it will include dependency updates, more work on hardware wallet support, updated dalek and friends, a rust update, and the fog view and ledger routers.

release/v4.1 is being released today. We have an hour set aside every week to discuss what goes into what releases and what the timelines are, and we've already pushed release/v4.1 back several times.

@cbeck88
Copy link
Contributor

cbeck88 commented Mar 15, 2023

Im not asking to push back release/v4.1 -- lets chat offline

It also may be fine to bump deqs to 5, im not sure. But id like to demo it soon against testnet and i expect there may be breaking changes in 5

@eranrund eranrund changed the base branch from release/v4.1 to master March 16, 2023 19:20
@eranrund eranrund merged commit d224eb6 into master Mar 16, 2023
@eranrund eranrund deleted the eran/admin-grpc-extra-services branch March 16, 2023 20:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants