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

Add configuration proto and start gRPC/HTTP servers using config #31

Merged
merged 7 commits into from
Oct 15, 2024

Conversation

mortenmj
Copy link
Collaborator

@mortenmj mortenmj commented Oct 14, 2024

This uses the convenience methods from the rest of the Buildbarn ecosystem to launch the gRPC and HTTP servers. I'd rather not refactor to use proto config for everything, as part of one large PR, so for now we are in a transitionary state where we still have some of the old flags in use.

Work towards #23

@trey-ivy
Copy link
Collaborator

thanks for doing this morten! I have some small nonblocking feedback. Also I'm fine if you want to combine this and #32 into a single PR. Seems cleaner to just cut over all the config to the jsonnet file at once, although at present I'm having issues building #32 locally.

Copy link
Collaborator

@trey-ivy trey-ivy left a comment

Choose a reason for hiding this comment

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

  • add a sample jsonnet file somewhere
  • update documentation on how to run
  • handle use case where config file isn't supplied and provide feedback msg
  • link to the open issue.

Copy link
Member

@EdSchouten EdSchouten left a comment

Choose a reason for hiding this comment

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

LGTM!

@mortenmj mortenmj merged commit 54bf0ca into main Oct 15, 2024
2 checks passed
@mortenmj mortenmj deleted the conf branch October 18, 2024 07:47
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.

3 participants