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

refactor: Move network servers under publish flags #1065

Closed
wants to merge 1 commit into from

Conversation

XAMPPRocky
Copy link
Collaborator

This is the first big step in my CLI refactoring, the goal is to move away from the distinct modes (agent, proxy, relay, etc) towards allowing users to specify individual components they want to enable (for example, to have the same functionality as the old proxy you would write --publish.udp --publish.qcmp --publish.phoenix). The intention here is to simplify both our code and the concepts around them and to make deployments more composable. This will reduce a lot of test surface because it will make writing tests for specific publishers much easier, and allow us to remove redundant code paths where we had to handle components that were shared between two modes but were unused by one mode.

Currently the modes are still included, because I wanted to divide the PR up, the intention is make a followup that applies the same logic to configuration providers and that will remove the modes entirely.

Copy link
Collaborator

@Jake-Shadle Jake-Shadle left a comment

Choose a reason for hiding this comment

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

Approving, though the publish term is not fully making sense to me.

@XAMPPRocky
Copy link
Collaborator Author

though the publish term is not fully making sense to me.

I thought about it in the sense of it being services that quilkin "exposes" or "publishes" to the user. Also in the sense of "config provider" being an input to quilkin, the "publisher" is output of quilkin. I was thinking about making config providers under a subscribe flag so it might feel more intuitive with those added. I am open to better suggestions though this was just me looking at a bunch of words and trying to find the one that matches the intent.

@XAMPPRocky XAMPPRocky force-pushed the ep/publish-cli branch 2 times, most recently from 6de690e to d432a54 Compare January 17, 2025 17:03
@Jake-Shadle
Copy link
Collaborator

No, that makes sense, I guess I always just first associate publish with like, releases/artifacts etc.

Copy link

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

Copy link

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

@XAMPPRocky XAMPPRocky enabled auto-merge (squash) January 20, 2025 11:53
Copy link

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

Copy link

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

1 similar comment
Copy link

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

Copy link

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

Copy link

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

Copy link

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

Copy link

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

Copy link

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

Copy link

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

Copy link

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

Copy link

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

Copy link

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

Copy link

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

Copy link

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

Copy link

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

Copy link

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

Copy link

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

Copy link

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

Copy link

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

Copy link

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

Copy link

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

@quilkin-bot
Copy link
Collaborator

Build Failed 😭

Build Id: 4a46ce53-ab56-4ebe-bb4d-39500f247f4c

Status: FAILURE

To get permission to view the Cloud Build view, join the quilkin-discuss Google Group.

Filter with the Git Commit 6ffd9a8 within us-docker.pkg.dev/quilkin/ci//quilkin to see if a development image is available from this build, and can be used for debugging purposes.

Development images are retained for at least 30 days.

@XAMPPRocky XAMPPRocky closed this Jan 22, 2025
auto-merge was automatically disabled January 22, 2025 19:11

Pull request was closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants