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

Decouple backend specific config from guacgql cmd #2247

Conversation

robert-cronin
Copy link
Contributor

@robert-cronin robert-cronin commented Oct 31, 2024

Description of the PR

Splitting out #2243 into two PRs, this one being the more aspirational change because it attempts to refactor more of the CLI towards modularity.

Some notes:

  • I intended to restrict this PR to refactoring only instead of altering functionality, so if there is any change to functionality it was not intentional, and we should probably flag and revert.
  • This PR attempts to decouple the backend implementations (including their arg parsing) from the core guacgql cmd code as a first step to better modularity.
  • The main change here is: a new mechanism to register the flags (config) of the backends alongside the existing function to register new backends. This takes the arg for any specific backend out of the guacgql cmd.

PR Checklist

  • All commits have a Developer Certificate of Origin (DCO) -- they are generated using -s flag to git commit.
  • All new changes are covered by tests
  • If GraphQL schema is changed, make generate has been run
  • If GraphQL schema is changed, GraphQL client updates/additions have been made
  • If OpenAPI spec is changed, make generate has been run
  • If ent schema is changed, make generate has been run
  • If collectsub protobuf has been changed, make proto has been run
  • All CI checks are passing (tests and formatting)
  • All dependent PRs have already been merged

@robert-cronin robert-cronin changed the title Refactor guacgql command to support out-of-tree backends Decouple backend specific config from guacgql cmd Oct 31, 2024
@robert-cronin robert-cronin force-pushed the feat/refactor-guacgql-for-external-backends branch 2 times, most recently from fd0a877 to f3345dd Compare November 17, 2024 23:32
@robert-cronin robert-cronin force-pushed the feat/refactor-guacgql-for-external-backends branch from f3345dd to a510df6 Compare November 18, 2024 00:35
@robert-cronin robert-cronin marked this pull request as ready for review November 18, 2024 02:03
@pxp928 pxp928 force-pushed the feat/refactor-guacgql-for-external-backends branch from a510df6 to 9fe75fd Compare November 20, 2024 12:45
Copy link
Collaborator

@mihaimaruseac mihaimaruseac left a comment

Choose a reason for hiding this comment

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

I like this!

Signed-off-by: robert-cronin <robert.owen.cronin@gmail.com>
@robert-cronin robert-cronin force-pushed the feat/refactor-guacgql-for-external-backends branch from 9fe75fd to 9c35c36 Compare November 21, 2024 01:33
Copy link
Collaborator

@jeffmendoza jeffmendoza left a comment

Choose a reason for hiding this comment

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

Very cool, thank you!

@kodiakhq kodiakhq bot merged commit 9982540 into guacsec:main Nov 25, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants