-
Notifications
You must be signed in to change notification settings - Fork 5
WIP: feature: update function to use admin server with headers #616
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
base: main
Are you sure you want to change the base?
Changes from all commits
9a7c865
5accefe
c308903
cccb039
8d2ca18
b19ea04
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -72,20 +72,45 @@ const getRequiredEnv = (name: string): string => { | |||||
| return value; | ||||||
| }; | ||||||
|
|
||||||
| type GraphQLClientOptions = { | ||||||
| hostHeaderEnvVar?: string; | ||||||
| databaseId?: string; | ||||||
| useMetaSchema?: boolean; | ||||||
| apiName?: string; | ||||||
| schemata?: string; | ||||||
| }; | ||||||
|
|
||||||
| // TODO: Consider moving this to @constructive-io/knative-job-fn as a shared | ||||||
| // utility so all job functions can create GraphQL clients with consistent | ||||||
| // header-based routing without duplicating this logic. | ||||||
|
Comment on lines
+83
to
+85
|
||||||
| const createGraphQLClient = ( | ||||||
| url: string, | ||||||
| hostHeaderEnvVar?: string | ||||||
| options: GraphQLClientOptions = {} | ||||||
| ): GraphQLClient => { | ||||||
Anmol1696 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
| const headers: Record<string, string> = {}; | ||||||
|
|
||||||
| if (process.env.GRAPHQL_AUTH_TOKEN) { | ||||||
| headers.Authorization = `Bearer ${process.env.GRAPHQL_AUTH_TOKEN}`; | ||||||
| } | ||||||
|
|
||||||
| const envName = hostHeaderEnvVar || 'GRAPHQL_HOST_HEADER'; | ||||||
| const envName = options.hostHeaderEnvVar || 'GRAPHQL_HOST_HEADER'; | ||||||
| const hostHeader = process.env[envName]; | ||||||
| if (hostHeader) { | ||||||
| headers.host = hostHeader; | ||||||
| headers.Host = hostHeader; | ||||||
|
||||||
| headers.Host = hostHeader; | |
| headers.host = hostHeader; |
Copilot
AI
Jan 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new header-based routing logic with multiple conditional header additions lacks test coverage. Consider adding unit tests to verify correct header construction for different option combinations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment states 'kept for future use' but GRAPHQL_API_NAME is set to 'private'. This is confusing - either the variable is currently used (in which case update the comment) or it's not needed yet (in which case consider removing it until actually required).