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

feat(prover): Add port opt for prover #2375

Closed
wants to merge 6 commits into from
Closed

Conversation

matias-gonz
Copy link
Collaborator

What ❔

Add port opt for prover

@matias-gonz matias-gonz marked this pull request as draft July 2, 2024 14:41
@Deniallugo Deniallugo requested a review from EmilLuta July 2, 2024 15:03
@EmilLuta
Copy link
Contributor

EmilLuta commented Jul 3, 2024

I understand this is still in draft, but will comment on the idea. I'm having a hard time understanding what is moved to file based config and what belongs to CLI. Maybe an overlying reasoning for it wouldn't be bad. (this goes to file, this goes to CLI and file -- maybe it already exists somewhere?).

My thought here is: I can totally understand why we want prometheus ports for things like WVG (when we test locally, we may start multiple of them), but then the rest are a bit weird (we run 1 per machine at all times in production and even locally).

Not opposing the PR, just trying to understand better the decisions.

Thanks in advance!

@matias-gonz
Copy link
Collaborator Author

Closed in favor of #2385

@matias-gonz matias-gonz closed this Jul 8, 2024
@matias-gonz matias-gonz deleted the matias-prover-port-cli branch July 8, 2024 09:39
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.

2 participants