Respect CLI arguments and configuration when both provided #342

ardatan · 2024-12-16T10:50:29Z

Closes #310

User can provide the token in the CLI arguments, and have some registry configuration in `gateway.config`

Since token can be provided in the CLI arguments, it should not be required in the configuration.

[Extra but needed] : Fixes in Test suite

Besides the changes in the code, I also saw that `bin` serve runner was not there and the tests were never running with the binary.
So I fixed it by adding bin serve runner implementation in the test suite.
I also found that process.env is not passed to the Docker runners as we do for other runners.

But I couldn't figure out why args not passed to the Docker runners, I need @enisdenjo 's help there because we test args here for Docker as in the original issue.

theguild-bot · 2024-12-17T07:58:03Z

🚀 Snapshot Release (`alpha`)

The latest changes of this PR are available as alpha on npm (based on the declared changesets):

Package	Version	Info
`@graphql-mesh/fusion-runtime`	`0.10.23-alpha-8ff78beeb027abbd7fe46dcb5238a2aa1e371f96`	npm ↗︎ unpkg ↗︎
`@graphql-hive/gateway`	`1.7.2-alpha-8ff78beeb027abbd7fe46dcb5238a2aa1e371f96`	npm ↗︎ unpkg ↗︎
`@graphql-mesh/plugin-opentelemetry`	`1.3.30-alpha-8ff78beeb027abbd7fe46dcb5238a2aa1e371f96`	npm ↗︎ unpkg ↗︎
`@graphql-mesh/plugin-prometheus`	`1.3.18-alpha-8ff78beeb027abbd7fe46dcb5238a2aa1e371f96`	npm ↗︎ unpkg ↗︎
`@graphql-hive/gateway-runtime`	`1.4.2-alpha-8ff78beeb027abbd7fe46dcb5238a2aa1e371f96`	npm ↗︎ unpkg ↗︎

theguild-bot · 2024-12-17T08:00:28Z

🚀 Snapshot Release (Binary for `macOS-ARM64`)

The latest changes of this PR are available for download (based on the declared changesets).

theguild-bot · 2024-12-17T08:00:30Z

🚀 Snapshot Release (Binary for `Linux-X64`)

The latest changes of this PR are available for download (based on the declared changesets).

theguild-bot · 2024-12-17T08:01:32Z

🚀 Snapshot Release (Bun Docker Image)

The latest changes of this PR are available as image on GitHub Container Registry (based on the declared changesets):

ghcr.io/graphql-hive/gateway:1.7.2-alpha-8ff78beeb027abbd7fe46dcb5238a2aa1e371f96-bun

theguild-bot · 2024-12-17T08:01:57Z

🚀 Snapshot Release (Node Docker Image)

The latest changes of this PR are available as image on GitHub Container Registry (based on the declared changesets):

ghcr.io/graphql-hive/gateway:1.7.2-alpha-8ff78beeb027abbd7fe46dcb5238a2aa1e371f96

theguild-bot · 2024-12-17T08:02:02Z

🚀 Snapshot Release (Binary for `macOS-X64`)

The latest changes of this PR are available for download (based on the declared changesets).

theguild-bot · 2024-12-17T08:06:28Z

🚀 Snapshot Release (Binary for `Windows-X64`)

The latest changes of this PR are available for download (based on the declared changesets).

Urigo · 2024-12-18T13:53:00Z

where can I find the documentation that covers both cases to pass the arguments?

ardatan · 2024-12-18T14:00:16Z

CLI Arguments reference -> https://the-guild.dev/graphql/hive/docs/api-reference/gateway-cli

We also have CLI configuration reference but it doesn't even mention about reporting;
https://the-guild.dev/graphql/hive/docs/api-reference/gateway-config

But instead we have this page for the configuration API for reporting
https://the-guild.dev/graphql/hive/docs/gateway/usage-reporting

And it is not clear how to pass other options like selfHosting :/

enisdenjo · 2024-12-18T14:19:17Z

packages/runtime/src/types.ts

-  /** GraphQL Hive registry access token. */
-  token: string;


There is no CLI or CLI arguments in the runtime. The token is still required in the runtime.

Then we need to make it optional for CLI only?

How about now? @enisdenjo

packages/gateway/src/config.ts

Urigo · 2024-12-18T16:29:51Z

CLI Arguments reference -> the-guild.dev/graphql/hive/docs/api-reference/gateway-cli

We also have CLI configuration reference but it doesn't even mention about reporting; the-guild.dev/graphql/hive/docs/api-reference/gateway-config

But instead we have this page for the configuration API for reporting the-guild.dev/graphql/hive/docs/gateway/usage-reporting

And it is not clear how to pass other options like selfHosting :/

Thanks!
Let's improve the docs around these as part of this PR (I understand its on a different repo...)

dotansimha · 2024-12-19T08:44:15Z

packages/gateway/src/commands/supergraph.ts

@@ -249,7 +250,9 @@ export const addCommand: AddCommand = (ctx, cli) =>
        process.exit(1);
      }
      return runSupergraph(ctx, config);
-    });
+    })
+    .allowUnknownOption(process.env.NODE_ENV === 'test')


Can you please elaborate on this change?

When running tests, NODE_ENV passed as test, and this allows us to accept extra CLI arguments.
We need this but why?
Because during tests random ports are used and the CLI needs to know which ports are used for which services.
Test suite passes those ports as arguments like --serviceName-port=13212
This was missing for GW CLI but only available for Mesh Compose. So in order to have a test self hosting service, we need to pass our test environment's port to the GW CLI.

dotansimha

LGTM. Let's wait for @enisdenjo confirmation.
@ardatan , do we need docs updates for this change? I

ardatan · 2024-12-19T11:14:46Z

Actually this is a PR for fixing a bug more than changing it. But I added docs for Hive Client reference here to make it clear for users need to use more features from Hive Console.

This is the PR improving docs for that purpose.
@dotansimha @Urigo
graphql-hive/console#6145

Co-authored-by: Denis Badurina <[email protected]>

This reverts commit 84b5cfc.

This reverts commit 73e8f28.

This reverts commit b175ad0.

ardatan mentioned this pull request Dec 16, 2024

Self-hosted usage reporting not working #310

Closed

ardatan force-pushed the test-self-hosting-hive branch from 0a3bbd0 to d1b889a Compare December 16, 2024 11:06

ardatan changed the title ~~Reproduction #310~~ Respect CLI arguments and configuration when both provided Dec 17, 2024

ardatan force-pushed the test-self-hosting-hive branch from f98bfd3 to 60f913c Compare December 17, 2024 07:58

ardatan force-pushed the test-self-hosting-hive branch from 60f913c to 8e74782 Compare December 17, 2024 08:49

ardatan marked this pull request as draft December 17, 2024 13:44

ardatan force-pushed the test-self-hosting-hive branch from d69118c to 4ff3aec Compare December 18, 2024 12:24

ardatan marked this pull request as ready for review December 18, 2024 12:24

ardatan force-pushed the test-self-hosting-hive branch from 4ff3aec to 6e1ea3c Compare December 18, 2024 12:39

enisdenjo requested changes Dec 18, 2024

View reviewed changes

ardatan force-pushed the test-self-hosting-hive branch from 3ba9b3c to 1f7a032 Compare December 19, 2024 08:22

dotansimha reviewed Dec 19, 2024

View reviewed changes

dotansimha approved these changes Dec 19, 2024

View reviewed changes

ardatan added 6 commits December 19, 2024 07:55

Reproduction #310

be512fa

..

86e27f3

NODE_ENV=test

fc2d74b

Format

5c3e3f4

Args?

64e5138

Less diff

9169a06

ardatan and others added 16 commits December 19, 2024 07:55

Align args

2714231

Add binary runner

c9c7923

Fullargs

a34bd4a

Update packages/gateway/src/config.ts

8763847

Co-authored-by: Denis Badurina <[email protected]>

Update config.ts

2d96745

Update config.ts

be04829

use docker hostname

bae665b

try on ci

d6a285d

pipe and cat

56d515e

if always

d3d5184

Revert "if always"

b405ae0

This reverts commit 84b5cfc.

Revert "pipe and cat"

83141e5

This reverts commit 73e8f28.

Revert "try on ci"

a53b531

This reverts commit b175ad0.

bun is also in docker

2bb7877

Token is optional on CLI but not on runtime

bb7e094

..

8ff78be

ardatan force-pushed the test-self-hosting-hive branch from 4b76032 to 8ff78be Compare December 19, 2024 12:55

ardatan requested a review from enisdenjo December 19, 2024 12:56

enisdenjo approved these changes Dec 19, 2024

View reviewed changes

ardatan merged commit 2f59fce into main Dec 19, 2024
34 of 36 checks passed

ardatan deleted the test-self-hosting-hive branch December 19, 2024 15:56

theguild-bot mentioned this pull request Dec 19, 2024

Upcoming Release Changes #354

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Respect CLI arguments and configuration when both provided #342

Respect CLI arguments and configuration when both provided #342

ardatan commented Dec 16, 2024 •

edited

Loading

theguild-bot commented Dec 17, 2024 •

edited

Loading

theguild-bot commented Dec 17, 2024 •

edited

Loading

theguild-bot commented Dec 17, 2024 •

edited

Loading

theguild-bot commented Dec 17, 2024 •

edited

Loading

theguild-bot commented Dec 17, 2024 •

edited

Loading

theguild-bot commented Dec 17, 2024 •

edited

Loading

theguild-bot commented Dec 17, 2024 •

edited

Loading

Urigo commented Dec 18, 2024

ardatan commented Dec 18, 2024

enisdenjo Dec 18, 2024

ardatan Dec 18, 2024

ardatan Dec 19, 2024

Urigo commented Dec 18, 2024

dotansimha Dec 19, 2024

ardatan Dec 19, 2024 •

edited

Loading

dotansimha left a comment

ardatan commented Dec 19, 2024 •

edited

Loading

Respect CLI arguments and configuration when both provided #342

Respect CLI arguments and configuration when both provided #342

Conversation

ardatan commented Dec 16, 2024 • edited Loading

[Extra but needed] : Fixes in Test suite

theguild-bot commented Dec 17, 2024 • edited Loading

🚀 Snapshot Release (alpha)

theguild-bot commented Dec 17, 2024 • edited Loading

🚀 Snapshot Release (Binary for macOS-ARM64)

theguild-bot commented Dec 17, 2024 • edited Loading

🚀 Snapshot Release (Binary for Linux-X64)

theguild-bot commented Dec 17, 2024 • edited Loading

🚀 Snapshot Release (Bun Docker Image)

theguild-bot commented Dec 17, 2024 • edited Loading

🚀 Snapshot Release (Node Docker Image)

theguild-bot commented Dec 17, 2024 • edited Loading

🚀 Snapshot Release (Binary for macOS-X64)

theguild-bot commented Dec 17, 2024 • edited Loading

🚀 Snapshot Release (Binary for Windows-X64)

Urigo commented Dec 18, 2024

ardatan commented Dec 18, 2024

enisdenjo Dec 18, 2024

Choose a reason for hiding this comment

ardatan Dec 18, 2024

Choose a reason for hiding this comment

ardatan Dec 19, 2024

Choose a reason for hiding this comment

Urigo commented Dec 18, 2024

dotansimha Dec 19, 2024

Choose a reason for hiding this comment

ardatan Dec 19, 2024 • edited Loading

Choose a reason for hiding this comment

dotansimha left a comment

Choose a reason for hiding this comment

ardatan commented Dec 19, 2024 • edited Loading

ardatan commented Dec 16, 2024 •

edited

Loading

theguild-bot commented Dec 17, 2024 •

edited

Loading

🚀 Snapshot Release (`alpha`)

theguild-bot commented Dec 17, 2024 •

edited

Loading

🚀 Snapshot Release (Binary for `macOS-ARM64`)

theguild-bot commented Dec 17, 2024 •

edited

Loading

🚀 Snapshot Release (Binary for `Linux-X64`)

theguild-bot commented Dec 17, 2024 •

edited

Loading

theguild-bot commented Dec 17, 2024 •

edited

Loading

theguild-bot commented Dec 17, 2024 •

edited

Loading

🚀 Snapshot Release (Binary for `macOS-X64`)

theguild-bot commented Dec 17, 2024 •

edited

Loading

🚀 Snapshot Release (Binary for `Windows-X64`)

ardatan Dec 19, 2024 •

edited

Loading

ardatan commented Dec 19, 2024 •

edited

Loading