Hive Console plugin for Gateway and introduce new target option #809
Conversation
|
Caution Review failedThe pull request is closed. 📝 WalkthroughSummary by CodeRabbit
WalkthroughThis pull request updates dependency management, refines reporting configuration handling, and introduces new functionality for Hive integrations. Key changes include updating dependencies in the runtime package, modifying how reporting configurations are processed across several modules, adding a new Hive console plugin, and enhancing CLI options for reporting. Additionally, documentation changes via changeset files reflect these updates. Changes
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (13)
✨ 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 (
|
🚀 Snapshot Release (
|
| Package | Version | Info |
|---|---|---|
@graphql-hive/gateway |
1.12.0-alpha-3aa59addc2562eb1e5c602a132ab458aec001ec0 |
npm ↗︎ unpkg ↗︎ |
@graphql-hive/nestjs |
1.0.1-alpha-3aa59addc2562eb1e5c602a132ab458aec001ec0 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/plugin-opentelemetry |
1.3.46-alpha-3aa59addc2562eb1e5c602a132ab458aec001ec0 |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/plugin-prometheus |
1.3.34-alpha-3aa59addc2562eb1e5c602a132ab458aec001ec0 |
npm ↗︎ unpkg ↗︎ |
@graphql-hive/gateway-runtime |
1.6.0-alpha-3aa59addc2562eb1e5c602a132ab458aec001ec0 |
npm ↗︎ unpkg ↗︎ |
🚀 Snapshot Release (Binary for
|
🚀 Snapshot Release (Binary for
|
🚀 Snapshot Release (Node Docker Image)The latest changes of this PR are available as image on GitHub Container Registry (based on the declared |
🚀 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
packages/gateway/src/commands/handleReportingConfig.ts (1)
79-100:⚠️ Potential issueRemove duplicate code at the end of the file.
There appears to be some duplicate code at the end of the file (lines 79-100) that seems to be a partial repetition of the Apollo GraphOS configuration logic. This code is likely a remnant from development or a mistake in the file and should be removed.
- target: opts.hiveUsageTarget, - }; - } - - if (opts.apolloKey) { - ctx.log.info(`Configuring Apollo GraphOS registry reporting`); - if (!opts.apolloGraphRef) { - ctx.log.error( - `Apollo GraphOS requires a graph ref in the format <graph-id>@<graph-variant>. Please provide a valid graph ref.`, - ); - process.exit(1); - } - return { - ...loadedConfig.reporting, - type: 'graphos', - apiKey: opts.apolloKey, - graphRef: opts.apolloGraphRef, - }; - } - - return null; -}
🧹 Nitpick comments (1)
packages/runtime/src/plugins/useHiveConsole.ts (1)
1-25: New plugin for Hive Console integrationThis new plugin implementation creates a bridge between the Gateway and Hive's reporting system. The code properly initializes the agent with sensible defaults, including the Gateway name and logger.
There's a noted TypeScript issue that should be addressed in a future update to ensure proper context type inheritance.
Consider adding a brief comment explaining the purpose of this plugin at the top of the file for better documentation.
+/** + * Plugin that enables Hive Console integration for the Gateway, + * providing usage reporting and other Hive features. + */ export default function useHiveConsole<
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (21)
packages/runtime/package.json(1 hunks)packages/runtime/src/createGatewayRuntime.ts(2 hunks)packages/runtime/src/getReportingPlugin.ts(2 hunks)packages/runtime/src/plugins/useHiveConsole.ts(1 hunks)packages/runtime/src/types.ts(2 hunks)packages/runtime/src/getReportingPlugin.ts(2 hunks)packages/runtime/src/types.ts(1 hunks)packages/gateway/src/cli.ts(2 hunks)packages/gateway/src/commands/handleReportingConfig.ts(1 hunks)packages/gateway/src/commands/proxy.ts(4 hunks)packages/gateway/src/commands/subgraph.ts(4 hunks)packages/gateway/src/commands/supergraph.ts(3 hunks)packages/gateway/src/commands/subgraph.ts(1 hunks)packages/gateway/src/commands/handleReportingConfig.ts(1 hunks)packages/gateway/src/commands/handleReportingConfig.ts(2 hunks)packages/runtime/src/createGatewayRuntime.ts(1 hunks)packages/runtime/src/getReportingPlugin.ts(1 hunks)packages/runtime/src/getReportingPlugin.ts(1 hunks).changeset/@graphql-hive_gateway-runtime-809-dependencies.md(1 hunks).changeset/plenty-llamas-push.md(1 hunks).changeset/wet-kiwis-peel.md(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
`packages/**`: In this directory we keep all packages releva...
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 changesetwith a full description on the changes with an example.
Also, when new features are added the packages, the changeset file should mentionminorsemver change. When a bug is fixed,patchneeds to be used. Themajorbump is used for a PR that has breaking changes.
packages/gateway/src/commands/subgraph.tspackages/runtime/src/getReportingPlugin.tspackages/gateway/src/commands/handleReportingConfig.tspackages/gateway/src/commands/supergraph.tspackages/runtime/src/createGatewayRuntime.tspackages/runtime/package.jsonpackages/runtime/src/types.tspackages/gateway/src/commands/proxy.tspackages/gateway/src/cli.tspackages/runtime/src/plugins/useHiveConsole.ts
`packages/gateway/**`: The main runtime for the Hive gateway...
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.
packages/gateway/src/commands/subgraph.tspackages/gateway/src/commands/handleReportingConfig.tspackages/gateway/src/commands/supergraph.tspackages/gateway/src/commands/proxy.tspackages/gateway/src/cli.ts
`**`: For all PRs, we would like to verify that a Linear iss...
**: 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
consoleproject.
Use the global knowledge feature to search for PRs ingraphql-hive/consolerepository that adds documentation. Suggest to create a follow up issue to add the missing documentation.
packages/gateway/src/commands/subgraph.tspackages/runtime/src/getReportingPlugin.tspackages/gateway/src/commands/handleReportingConfig.tspackages/gateway/src/commands/supergraph.tspackages/runtime/src/createGatewayRuntime.tspackages/runtime/package.jsonpackages/runtime/src/types.tspackages/gateway/src/commands/proxy.tspackages/gateway/src/cli.tspackages/runtime/src/plugins/useHiveConsole.ts
⏰ Context from checks skipped due to timeout of 90000ms (43)
- GitHub Check: Examples / Convert operation-field-permissions
- GitHub Check: Examples / Convert type-merging-batching
- GitHub Check: Examples / Convert subscriptions-with-transforms
- GitHub Check: Examples / Convert programmatic-batching
- GitHub Check: Examples / Convert openapi-subscriptions
- GitHub Check: Examples / Convert openapi-javascript-wiki
- GitHub Check: Examples / Convert openapi-arg-rename
- GitHub Check: Examples / Convert openapi-additional-resolvers
- GitHub Check: Examples / Convert json-schema-subscriptions
- GitHub Check: Examples / Convert interface-additional-resolvers
- GitHub Check: Examples / Convert hmac-auth-https
- GitHub Check: Examples / Convert federation-subscriptions-passthrough
- GitHub Check: Examples / Convert apq-subgraphs
- GitHub Check: Examples / Convert file-upload
- GitHub Check: Examples / Convert federation-mixed
- GitHub Check: Examples / Convert federation-example
- GitHub Check: Examples / Convert extra-fields
- GitHub Check: Binary built on ubuntu-24.04-arm
- GitHub Check: Binary built on windows-latest
- GitHub Check: Binary built on macos-14
- GitHub Check: Binary built on macos-13
- GitHub Check: Bun Docker image
- GitHub Check: Binary built on ubuntu-latest
- GitHub Check: Node Docker image
- GitHub Check: E2E / Bun Docker on Ubuntu
- GitHub Check: E2E / Bun on Ubuntu
- GitHub Check: E2E / Node Binary on Windows
- GitHub Check: E2E / Node Binary on Ubuntu
- GitHub Check: E2E / Node Docker on Ubuntu
- GitHub Check: E2E / Node 23 on Ubuntu
- GitHub Check: E2E / Node 22 on Ubuntu
- GitHub Check: Benchmark / bun / 1000 items
- GitHub Check: Benchmark / bun / 100 items
- GitHub Check: Leaks / Node v23
- GitHub Check: Benchmark / bun / 10 items
- GitHub Check: Benchmark / node / 1000 items
- GitHub Check: E2E / Node 20 on Ubuntu
- GitHub Check: Benchmark / node / 100 items
- GitHub Check: Benchmark / node / 10 items
- GitHub Check: Leaks / Node v22
- GitHub Check: E2E / Node 18 on Ubuntu
- GitHub Check: Leaks / Node v20
- GitHub Check: Leaks / Node v18
🔇 Additional comments (23)
.changeset/wet-kiwis-peel.md (1)
1-6: LGTM: The changeset correctly declares a patch version bump for the new featureThe changeset properly documents the introduction of the new
targetreporting option for the@graphql-hive/gateway-runtimepackage with a patch version bump, which is appropriate for a non-breaking addition..changeset/plenty-llamas-push.md (1)
1-7: LGTM: Properly documented changeset with clear deprecation noticeThe changeset correctly documents both the new
targetreporting option and the CLI options deprecation for the@graphql-hive/gatewaypackage with appropriate patch versioning..changeset/@graphql-hive_gateway-runtime-809-dependencies.md (1)
1-9: LGTM: Dependencies changeset properly documentedThe changeset correctly documents the dependency changes (adding
@graphql-hive/yogaand removing@graphql-mesh/plugin-hive) with a patch version bump, which is appropriate for dependency updates.packages/runtime/package.json (1)
52-52: LGTM: Dependency update matches changeset declarationThe addition of
@graphql-hive/yoga@^0.41.0aligns with the changeset file that documents this dependency change.packages/runtime/src/types.ts (1)
274-275: LGTM: New optional property with clear documentationThe
targetproperty is properly added to theGatewayHiveReportingOptionsinterface with good documentation and marked as optional, which preserves backward compatibility.packages/runtime/src/createGatewayRuntime.ts (1)
194-197: Improved configuration handling for reporting pluginThe change properly passes the configuration context to the reporting plugin, ensuring it has access to logger, cache, and other contextual information. The removal of the empty object fallback (
|| {}) suggests thatgetReportingPluginnow consistently returns a structured object.packages/gateway/src/commands/supergraph.ts (3)
31-31: New import for centralized reporting configuration handlingThis import brings in the
handleReportingConfigutility function that centralizes the reporting configuration logic.
54-56: Added new CLI options for Hive usage reportingTwo new options
hiveUsageTargetandhiveUsageAccessTokenhave been added to support the new Hive usage reporting mechanism.
157-167:✅ Verification successful
Refactored reporting configuration to use centralized utility function
The code now uses the
handleReportingConfigutility function to process various reporting-related options, including both the legacyhiveRegistryTokenand the newhiveUsageTarget/hiveUsageAccessTokenoptions. This centralization makes the code more maintainable and consistent across different commands.
🏁 Script executed:
#!/bin/bash # Check for a changeset file that documents this change fd ".*\.md" .changeset -x cat {}Length of output: 22595
Refactored Reporting Configuration Verified and Approved
The refactoring in
packages/gateway/src/commands/supergraph.ts(lines 157–167) correctly uses the centralizedhandleReportingConfigfunction to process both the legacyhiveRegistryTokenoption and the newhiveUsageTarget/hiveUsageAccessTokenoptions. This update is also clearly documented in the changeset (e.g., "Introducetargetas a new Hive reporting option" and the deprecation of--hive-registry-tokenin favor of the new options), ensuring that the intended changes are both implemented and tracked.
- Location:
packages/gateway/src/commands/supergraph.ts, lines 157–167- Impact: Centralizes and streamlines reporting configuration for improved maintainability and consistency.
packages/runtime/src/getReportingPlugin.ts (1)
21-28: Improved handling of Hive usage configurationThe updated logic properly handles the case where usage reporting is explicitly disabled (
usage === false). For other cases, it now correctly extends the usage configuration with the target property, ensuring type safety by only spreading properties ifusageis an object.packages/gateway/src/commands/subgraph.ts (4)
25-25: Good addition of handleReportingConfig import.This import supports the centralized approach to reporting configuration handling that's being implemented across the codebase.
41-42: New CLI options for Hive usage properly integrated.The addition of
hiveUsageTargetandhiveUsageAccessTokenoptions aligns with the updated CLI interface defined in cli.ts, providing more flexible configuration for Hive reporting.
61-72: Well-structured reporting configuration handling.The implementation centralizes the reporting configuration by using the
handleReportingConfigfunction, which improves code organization and maintainability. The approach correctly preserves any additional reporting configuration from the loaded config.
107-107: Good use of spread operator for registryConfig.Using the spread operator to incorporate the registryConfig into the final config object is clean and maintains consistency with the other configuration sections.
packages/gateway/src/commands/proxy.ts (4)
21-21: Good addition of handleReportingConfig import.This import supports the centralized approach to reporting configuration handling that's being implemented across the codebase.
39-40: New CLI options for Hive usage properly integrated.The addition of
hiveUsageTargetandhiveUsageAccessTokenoptions aligns with the updated CLI interface defined in cli.ts, providing more flexible configuration for Hive reporting.
101-112: Well-structured reporting configuration handling.The implementation centralizes the reporting configuration by using the
handleReportingConfigfunction, which improves code organization and maintainability. The approach correctly preserves any additional reporting configuration from the loaded config.
147-147: Good use of spread operator for registryConfig.Using the spread operator to incorporate the registryConfig into the final config object is clean and maintains consistency with the other configuration sections.
packages/gateway/src/cli.ts (3)
75-82: Updated interface to support new Hive reporting options.The interface correctly omits
targetandtokenproperties from the base interface and redefines them with updated documentation. The documentation clearly specifies the default values from environment variables.
318-319: Good deprecation notice for hive-registry-token.The deprecation message clearly instructs users on the new options to use instead of the deprecated one.
321-332: Well-defined new CLI options for Hive usage reporting.The new options are properly defined with clear descriptions and environment variable fallbacks. The descriptions correctly indicate that both options are required when using either one.
packages/gateway/src/commands/handleReportingConfig.ts (2)
1-78: Good implementation of centralized reporting configuration handling.The
handleReportingConfigfunction is well-structured and handles different reporting configuration scenarios appropriately:
- It prevents mixing old and new configuration methods with clear error messages
- Provides a deprecation warning when using the old
hiveRegistryTokenoption- Ensures both
hiveUsageTargetandhiveUsageAccessTokenare provided when using the new approach- Properly configures Apollo GraphOS reporting when those options are provided
This centralization makes the code more maintainable and keeps the logic consistent across different commands.
1-78:❓ Verification inconclusive
Verify that a changeset file has been created.
According to the project's coding guidelines, when new features are added to packages, a changeset file should be created using
yarn changesetwith a full description and aminorsemver change. Since this PR introduces new CLI options and deprecates an existing one, please ensure a changeset file has been included.
🏁 Script executed:
#!/bin/bash # Description: Check if a changeset file exists for this PR # Look for changeset files echo "Looking for changeset files..." find .changeset -name "*.md" | grep -v README.mdLength of output: 4187
Changeset Verification:
The repository already contains changeset files; however, please double-check that the changeset file addressing this PR’s new CLI options and deprecation (i.e. not one of the dependency updates) includes a full description and marks a minor semver change as per our guidelines.
🚀 Snapshot Release (Binary for
|
ardatan
force-pushed
the
use-hive-console
branch
from
04952b7 to
6451d25
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
packages/runtime/src/getReportingPlugin.ts (1)
1-91: 💡 Verification agent🧩 Analysis chain
Verify changeset file creation
A changeset file should be created for these changes using
yarn changesetwith a full description of the new functionality and examples. Since this PR introduces new features (new target option), the changeset should specify a "minor" semver change.
🏁 Script executed:
#!/bin/bash # Check if a changeset file was created for this PR echo "Checking for changeset files..." fd -e md -p '.changeset/'Length of output: 98
Action Required: Create a Changeset File
It appears that no changeset file was found in the
.changeset/directory. Since this PR introduces a new target option feature, please runyarn changesetto generate a changeset file. Make sure the file includes:
- A full description of the new functionality (e.g., the new target option).
- Usage examples as needed.
- A minor semver bump designation.
Please add this changeset file before merging.
🧹 Nitpick comments (7)
packages/runtime/src/getReportingPlugin.ts (1)
1-91: Documentation for new CLI optionsSince this PR introduces new CLI options (
--hive-usage-targetand--hive-usage-access-token), documentation should be added to the corresponding console project.Would you like me to create a follow-up issue to ensure documentation is added for these new CLI options?
packages/gateway/src/commands/supergraph.ts (1)
157-166: Add tests for the new reporting flow.The centralized call to
handleReportingConfigis clear and maintainable. However, consider adding or extending tests to validate usage reporting or registry reporting logic (especially with the new CLI options).packages/gateway/src/commands/subgraph.ts (1)
61-72: Subgraph restricted to Hive reporting.Explicitly disabling GraphOS by setting
apolloGraphRefandapolloKeytoundefinedmakes sense if that’s the expected behavior. The newregistryConfigapproach is consistent with the supergraph. Please ensure there are tests or validation for subgraph usage reporting using Hive.Also applies to: 107-107
packages/gateway/src/commands/handleReportingConfig.ts (2)
38-64: Conflict checks are well-defined.Exiting early for conflicting or incomplete options is good. However, consider adding tests specifically verifying these error paths.
66-81: Hive usage logic is correct and consistent.Reusing
hiveRegistryTokenas a fallback forhiveUsageAccessTokenstreamlines backward compatibility. Confirm that references to the deprecated--hive-registry-tokenare documented or guided for migration to the new usage tokens.packages/gateway/src/commands/proxy.ts (2)
39-40: New CLI options added for Hive usage reporting.These new options align with the PR objective to introduce
--hive-usage-targetand--hive-usage-access-token. However, there's no explicit deprecation notice forhiveRegistryTokenwhich is mentioned as deprecated in the PR objectives.Consider adding a comment above
hiveRegistryTokenin the destructuring assignment to indicate its deprecated status, such as:hiveCdnEndpoint, hiveCdnKey, + // @deprecated - Use hiveUsageTarget and hiveUsageAccessToken instead hiveRegistryToken, hiveUsageTarget, hiveUsageAccessToken,
101-112: Refactored reporting configuration with improved type safety.The refactoring centralizes the reporting configuration logic and improves type safety with
Pick<ProxyConfig, 'reporting'>. The implementation correctly handles the new options while maintaining backward compatibility withhiveRegistryToken.Consider adding a brief comment explaining the purpose of this configuration block:
+ // Configure Hive reporting options for usage tracking const registryConfig: Pick<ProxyConfig, 'reporting'> = {}; const reporting = handleReportingConfig(ctx, loadedConfig, {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (13)
.changeset/@graphql-hive_gateway-runtime-809-dependencies.md(1 hunks).changeset/plenty-llamas-push.md(1 hunks).changeset/wet-kiwis-peel.md(1 hunks)packages/gateway/src/cli.ts(2 hunks)packages/gateway/src/commands/handleReportingConfig.ts(1 hunks)packages/gateway/src/commands/proxy.ts(4 hunks)packages/gateway/src/commands/subgraph.ts(4 hunks)packages/gateway/src/commands/supergraph.ts(3 hunks)packages/runtime/package.json(1 hunks)packages/runtime/src/createGatewayRuntime.ts(2 hunks)packages/runtime/src/getReportingPlugin.ts(2 hunks)packages/runtime/src/plugins/useHiveConsole.ts(1 hunks)packages/runtime/src/types.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- .changeset/wet-kiwis-peel.md
- .changeset/plenty-llamas-push.md
- .changeset/@graphql-hive_gateway-runtime-809-dependencies.md
- packages/runtime/package.json
- packages/runtime/src/types.ts
- packages/runtime/src/plugins/useHiveConsole.ts
- packages/gateway/src/cli.ts
🧰 Additional context used
📓 Path-based instructions (3)
`packages/**`: In this directory we keep all packages releva...
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 changesetwith a full description on the changes with an example.
Also, when new features are added the packages, the changeset file should mentionminorsemver change. When a bug is fixed,patchneeds to be used. Themajorbump is used for a PR that has breaking changes.
packages/gateway/src/commands/proxy.tspackages/gateway/src/commands/supergraph.tspackages/gateway/src/commands/handleReportingConfig.tspackages/runtime/src/createGatewayRuntime.tspackages/runtime/src/getReportingPlugin.tspackages/gateway/src/commands/subgraph.ts
`packages/gateway/**`: The main runtime for the Hive gateway...
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.
packages/gateway/src/commands/proxy.tspackages/gateway/src/commands/supergraph.tspackages/gateway/src/commands/handleReportingConfig.tspackages/gateway/src/commands/subgraph.ts
`**`: For all PRs, we would like to verify that a Linear iss...
**: 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
consoleproject.
Use the global knowledge feature to search for PRs ingraphql-hive/consolerepository that adds documentation. Suggest to create a follow up issue to add the missing documentation.
packages/gateway/src/commands/proxy.tspackages/gateway/src/commands/supergraph.tspackages/gateway/src/commands/handleReportingConfig.tspackages/runtime/src/createGatewayRuntime.tspackages/runtime/src/getReportingPlugin.tspackages/gateway/src/commands/subgraph.ts
⏰ Context from checks skipped due to timeout of 90000ms (26)
- GitHub Check: Unit / Node v23
- GitHub Check: Unit / Bun
- GitHub Check: Unit / Node v22
- GitHub Check: Unit / Node v20
- GitHub Check: Leaks / Node v23
- GitHub Check: Leaks / Node v22
- GitHub Check: Leaks / Node v20
- GitHub Check: Leaks / Node v18
- GitHub Check: E2E / Bun Docker on Ubuntu
- GitHub Check: E2E / Bun on Ubuntu
- GitHub Check: E2E / Node Binary on Windows
- GitHub Check: E2E / Node Binary on Ubuntu
- GitHub Check: E2E / Node Docker on Ubuntu
- GitHub Check: Benchmark / bun / 1000 items
- GitHub Check: E2E / Node 23 on Ubuntu
- GitHub Check: Snapshot / snapshot
- GitHub Check: Benchmark / bun / 100 items
- GitHub Check: E2E / Node 22 on Ubuntu
- GitHub Check: Benchmark / bun / 10 items
- GitHub Check: Benchmark / node / 1000 items
- GitHub Check: E2E / Node 20 on Ubuntu
- GitHub Check: Bundle
- GitHub Check: Build
- GitHub Check: E2E / Node 18 on Ubuntu
- GitHub Check: Benchmark / node / 100 items
- GitHub Check: Benchmark / node / 10 items
🔇 Additional comments (18)
packages/runtime/src/createGatewayRuntime.ts (2)
98-98: New import for Hive Console pluginThis import correctly replaces the previous Mesh Hive plugin import, aligning with the PR objective of relocating the Hive plugin from Mesh to Gateway.
205-208: Good implementation of the new Hive Console pluginThe
persistedDocumentsPluginnow properly uses the newuseHiveConsolefunction instead of the previous Mesh-based plugin. Settingenabled: falseensures that only usage reporting is disabled while persisted documents functionality remains intact.packages/runtime/src/getReportingPlugin.ts (4)
2-4: Updated imports for Hive Console pluginThe import statement correctly brings in the new Hive Console plugin and its types, which is necessary for the plugin migration described in the PR objectives.
15-15: Updated return type to support GraphOSThe return type now correctly allows the name to be either 'Hive' or 'GraphOS', providing better type safety.
19-29: Improved handling of target and usage configurationThis code change adds support for the new
targetoption mentioned in the PR objectives. The implementation correctly handles cases where usage is explicitly disabled, and properly constructs the usage object when target is specified.The type check before spreading usage properties is a good defensive programming practice to avoid errors when trying to spread non-object values.
32-37: Updated plugin initializationThe code now properly uses the new
useHiveConsoleplugin withenabled: trueto ensure usage reporting is activated when configured. The conditional spreading of the usage object ensures that configuration is only applied when relevant.packages/gateway/src/commands/supergraph.ts (2)
31-31: Well-structured import for reporting configuration.No issues found; extracting reporting logic into a dedicated helper module improves maintainability and readability.
54-55: Ensure changeset includes new CLI options.You've introduced
--hive-usage-targetand--hive-usage-access-token. According to our guidelines, please confirm there's a corresponding changeset describing these new CLI options with a minor semver bump.packages/gateway/src/commands/subgraph.ts (2)
25-25: Import statement looks good.Centralizing logic for reporting configuration enhances modularity.
41-42: Include these new subgraph usage flags in the changeset.Remember to confirm that the subgraph’s newly accepted CLI options are documented, consistent with supergraph usage, and covered in the existing or new changeset file.
packages/gateway/src/commands/handleReportingConfig.ts (5)
1-6: Imports are concise.Everything is well-organized and matches the needed types from the parent directories.
8-14: Interface definition is clear.The new fields for Hive usage target and access token are straightforward. Ensure these remain in sync with any further additions to the CLI flags.
16-37: Centralizing reporting configuration.Overall approach merges config-based reporting with CLI overrides. This simplifies the logic in the command files. Great job!
83-97: GraphOS reporting configuration looks good.Apollo usage explicitly requires a graph ref in
<graph-id>@<graph-variant>format. Good job enforcing that requirement and cleaning up the command-level logic.
99-100: Handling no configuration scenario gracefully.Returning
nullwhen no valid reporting options apply is clear. Make sure the calling code checks for a null return properly (which you have done).packages/gateway/src/commands/proxy.ts (3)
21-21: Approve module import for centralized reporting config handling.Good modularization by centralizing reporting configuration logic into a separate file.
147-147: Simplified configuration composition using spread operator.Clean implementation using the spread operator to merge the reporting configuration.
34-196:❓ Verification inconclusive
Verify documentation for new CLI options.
The PR adds two new CLI options (
--hive-usage-targetand--hive-usage-access-token) but they aren't documented in the CLI help text. While the implementation is sound, users won't know about these options without proper documentation.Could you verify if documentation for these new options has been added to the corresponding console project as mentioned in the coding guidelines? If not, consider creating a follow-up issue to add documentation for these new options and the deprecation of
--hive-registry-token.
🏁 Script executed:
#!/bin/bash # Check if the new CLI options are documented in the command grep -r --include="*.ts" "hiveUsageTarget\|hiveUsageAccessToken" packages/Length of output: 2346
Documentation Update for New CLI Options Required
The new CLI options (
--hive-usage-targetand--hive-usage-access-token) are properly integrated into the code, as visible in the reporting configuration. However, there's no evidence from the repository that these options have been added to the corresponding console project's CLI help text. Please verify whether the documentation update has been completed per our coding guidelines. If not, create a follow-up issue to add the missing documentation for these new options and to note the deprecation of--hive-registry-token.
enisdenjo
force-pushed
the
use-hive-console
branch
from
6451d25 to
3aa59ad
Compare
Ref GW-240
Docs: graphql-hive/console#6612.
Waiting for graphql-hive/console#6591.
--hive-usage-targetand--hive-usage-access-tokenCLI and configuration options--hive-registry-tokenand advise user to migrate