fix(cli): override CLI defaults from configuration #574
Conversation
|
add a changeset? @ardatan |
|
@coderabbitai full review |
📝 WalkthroughSummary by CodeRabbitRelease Notes
WalkthroughThe pull request introduces modifications to the Hive Gateway's configuration and command-line interface (CLI) handling, particularly concerning the Changes
Assessment against linked issues
The changes do not appear to resolve the reported issue with port configuration, where the gateway is not respecting the specified port in the configuration. While the polling interval configuration has been improved, the core problem of port selection remains unaddressed in these changes. Possibly related PRs
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.changeset/dull-mayflies-allow.md (1)
5-5: Improve the description's grammar and clarity.The description has a minor grammatical issue and could be more specific.
Apply this diff to improve the description:
-Fix the regression causing `port`, `host` and `pollingInterval` in the configuration is overriden by the default values of CLI parameters +Fix a regression where `port`, `host`, and `pollingInterval` configuration values were being overridden by CLI default parameters
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.changeset/dull-mayflies-allow.md(1 hunks)
🔇 Additional comments (1)
.changeset/dull-mayflies-allow.md (1)
1-3: LGTM! Appropriate version bump.The patch version bump is correct as this fixes a regression without introducing breaking changes.
|
@coderabbitai full review |
🚀 Snapshot Release (Binary for
|
🚀 Snapshot Release (Binary for
|
🚀 Snapshot Release (Binary for
|
🚀 Snapshot Release (Bun Docker Image)The latest changes of this PR are available as image on GitHub Container Registry (based on the declared |
🚀 Snapshot Release (Node Docker Image)The latest changes of this PR are available as image on GitHub Container Registry (based on the declared |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/gateway/src/commands/supergraph.ts (1)
209-214: LGTM! Consider adding type guard for better type safety.The fallback chain for
pollingIntervalcorrectly implements the CLI defaults override from configuration. The implementation follows a clear precedence: CLI options > config file > default values.Consider adding a type guard to make the configuration type checking more explicit:
pollingInterval: opts.polling || - ('pollingInterval' in loadedConfig + (loadedConfig && 'pollingInterval' in loadedConfig ? loadedConfig.pollingInterval : undefined) || defaultOptions.pollingInterval,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.changeset/dull-mayflies-allow.md(1 hunks)e2e/polling/gateway.config.ts(0 hunks)packages/gateway/src/cli.ts(3 hunks)packages/gateway/src/commands/proxy.ts(2 hunks)packages/gateway/src/commands/subgraph.ts(2 hunks)packages/gateway/src/commands/supergraph.ts(2 hunks)
💤 Files with no reviewable changes (1)
- e2e/polling/gateway.config.ts
🧰 Additional context used
📓 Path-based instructions (4)
packages/gateway/src/commands/proxy.ts (3)
Pattern packages/**: In this directory we keep all packages relevant to the gateway.
In most cases, when changes are made to the source code / dependencies in these directories, we also require to have a corresponding changeset file, created using yarn changeset with a full description on the changes with an example.
Also, when new features are added the packages, the changeset file should mention minor semver change. When a bug is fixed, patch needs to be used. The major bump is used for a PR that has breaking changes.
Pattern packages/gateway/**: The main runtime for the Hive gateway.
This package is CLI that runs the gateway and configures the internals of the gateway function.
Pattern **: For all PRs, we would like to verify that a Linear issue is linked. Check if there's a mention for "GW-*" in the PR description, and the us know when it's missing.
In most cases, when a PR adds a new functionality, we would like to make sure it has documentation in the corresponding console project.
Use the global knowledge feature to search for PRs in graphql-hive/console repository that adds documentation. Suggest to create a follow up issue to add the missing documentation.
packages/gateway/src/commands/supergraph.ts (3)
Pattern packages/**: In this directory we keep all packages relevant to the gateway.
In most cases, when changes are made to the source code / dependencies in these directories, we also require to have a corresponding changeset file, created using yarn changeset with a full description on the changes with an example.
Also, when new features are added the packages, the changeset file should mention minor semver change. When a bug is fixed, patch needs to be used. The major bump is used for a PR that has breaking changes.
Pattern packages/gateway/**: The main runtime for the Hive gateway.
This package is CLI that runs the gateway and configures the internals of the gateway function.
Pattern **: For all PRs, we would like to verify that a Linear issue is linked. Check if there's a mention for "GW-*" in the PR description, and the us know when it's missing.
In most cases, when a PR adds a new functionality, we would like to make sure it has documentation in the corresponding console project.
Use the global knowledge feature to search for PRs in graphql-hive/console repository that adds documentation. Suggest to create a follow up issue to add the missing documentation.
packages/gateway/src/cli.ts (3)
Pattern packages/**: In this directory we keep all packages relevant to the gateway.
In most cases, when changes are made to the source code / dependencies in these directories, we also require to have a corresponding changeset file, created using yarn changeset with a full description on the changes with an example.
Also, when new features are added the packages, the changeset file should mention minor semver change. When a bug is fixed, patch needs to be used. The major bump is used for a PR that has breaking changes.
Pattern packages/gateway/**: The main runtime for the Hive gateway.
This package is CLI that runs the gateway and configures the internals of the gateway function.
Pattern **: For all PRs, we would like to verify that a Linear issue is linked. Check if there's a mention for "GW-*" in the PR description, and the us know when it's missing.
In most cases, when a PR adds a new functionality, we would like to make sure it has documentation in the corresponding console project.
Use the global knowledge feature to search for PRs in graphql-hive/console repository that adds documentation. Suggest to create a follow up issue to add the missing documentation.
packages/gateway/src/commands/subgraph.ts (3)
Pattern packages/**: In this directory we keep all packages relevant to the gateway.
In most cases, when changes are made to the source code / dependencies in these directories, we also require to have a corresponding changeset file, created using yarn changeset with a full description on the changes with an example.
Also, when new features are added the packages, the changeset file should mention minor semver change. When a bug is fixed, patch needs to be used. The major bump is used for a PR that has breaking changes.
Pattern packages/gateway/**: The main runtime for the Hive gateway.
This package is CLI that runs the gateway and configures the internals of the gateway function.
Pattern **: For all PRs, we would like to verify that a Linear issue is linked. Check if there's a mention for "GW-*" in the PR description, and the us know when it's missing.
In most cases, when a PR adds a new functionality, we would like to make sure it has documentation in the corresponding console project.
Use the global knowledge feature to search for PRs in graphql-hive/console repository that adds documentation. Suggest to create a follow up issue to add the missing documentation.
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: Binary built on windows-latest
- GitHub Check: Binary built on macos-13
- GitHub Check: Bun Docker image
- GitHub Check: Node Docker image
- GitHub Check: E2E / Bun Docker on Ubuntu
- GitHub Check: E2E / Node Binary on Windows
- GitHub Check: E2E / Node Docker on Ubuntu
🔇 Additional comments (10)
packages/gateway/src/commands/subgraph.ts (2)
85-90: LGTM! Proper fallback mechanism for pollingInterval.The implementation correctly prioritizes values from different sources:
- CLI option (
opts.polling)- Configuration file (
loadedConfig.pollingInterval)- Default value (
defaultOptions.pollingInterval)
143-143: LGTM! Improved error message.The error message now includes the actual polling interval value, making it easier to debug configuration issues.
packages/gateway/src/commands/proxy.ts (2)
124-129: LGTM! Consistent implementation of pollingInterval fallback.The implementation matches the pattern used in
subgraph.ts, ensuring consistent behavior across commands.
183-183: LGTM! Consistent error message improvement.The error message update matches the improvement made in
subgraph.ts.packages/gateway/src/cli.ts (3)
228-228: LGTM! Improved help text for --fork option.The help text now clearly indicates the default value and its behavior in different environments.
251-253: LGTM! Improved help text for host and port options.The help text now includes the default values, making the configuration behavior more transparent.
Also applies to: 257-260
273-273: LGTM! Improved help text for polling option.The help text now uses a human-readable duration format for better clarity.
.changeset/dull-mayflies-allow.md (1)
1-5: LGTM! Clear and accurate changeset.The changeset correctly:
- Uses a patch version bump for a bug fix
- Clearly describes the regression being fixed
packages/gateway/src/commands/supergraph.ts (2)
259-259: LGTM! Improved error message clarity.The error message now includes the actual polling interval value, making it easier for users to understand why their configuration was rejected.
209-214: Add a changeset file for the package changes.Since this PR fixes a bug in the gateway package, a changeset file needs to be created using
yarn changesetwith apatchsemver change. Please include a description of the changes with an example.Run the following script to check if a changeset file exists:
✅ Verification successful
Changeset requirement is already satisfied ✓
A changeset file exists with the appropriate patch version bump and clear description of the bug fix for the gateway package.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for changeset files in the PR # Look for .md files in .changeset directory fd -e md . .changeset/Length of output: 58
Script:
#!/bin/bash # Check contents of the changeset file cat .changeset/dull-mayflies-allow.mdLength of output: 217
🚀 Snapshot Release (Binary for
|
🚀 Snapshot Release (Binary for
|
Fixes #573
Ref GW-136