Make graphql urls conditional by peppermint-juli · Pull Request #89 · sciserver/opensciserver

peppermint-juli · 2026-02-13T18:12:41Z

Environment-specific configuration:

Updated the graphql-deployment.yaml Helm template to set service base URLs (FILES_BASE_URL, COMPUTE_BASE_URL, RACM_BASE_URL, LOGIN_PORTAL_BASE_URL) to https://apps.sciserver.org/ when running in the dev environment, and to the usual values otherwise. This ensures the correct endpoints are used depending on the deployment environment.

GraphQL server setup simplification:

Removed the use of addMocksToSchema and related mocking logic from main.ts, so the server always uses the real schema and resolvers regardless of environment. This reduces complexity and avoids unnecessary test-only code in production and development. [1] [2]

…le URLs for deployment

Copilot

Pull request overview

This PR updates the SciServer GraphQL deployment configuration to use environment-specific upstream service URLs, and simplifies the GraphQL server bootstrap to always use the real schema/resolvers (removing test-only mocking).

Changes:

Updated the GraphQL Helm Deployment template to conditionally set FILES_BASE_URL, COMPUTE_BASE_URL, RACM_BASE_URL, and LOGIN_PORTAL_BASE_URL based on .Values.graphql.env.
Removed addMocksToSchema usage so the Apollo Server always runs with the real schema.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
`helm/sciserver/templates/graphql/graphql-deployment.yaml`	Adds environment-conditioned base URLs for downstream services used by GraphQL.
`components/ui/graphql/src/main.ts`	Removes test-only schema mocking and keeps a single real schema configuration.

Comments suppressed due to low confidence (1)

components/ui/graphql/src/main.ts:50

The landing page plugin toggle checks process.env.IDIES_ENV !== 'production', but the rest of this repo/Helm values use dev/prod (e.g., environment.ts checks for 'prod' and the chart sets IDIES_ENV from .Values.graphql.env). As written, the landing page will remain enabled even when IDIES_ENV=prod, which is likely unintended for production. Align this comparison with the actual environment values used by the chart/app.

  plugins: process.env.IDIES_ENV !== 'production' ?
    [
      // eslint-disable-next-line new-cap
      ApolloServerPluginLandingPageLocalDefault({ footer: false })
    ]
    : [],

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-13T18:16:14Z

components/ui/graphql/src/main.ts

 import { ApolloServer } from '@apollo/server';
 import { expressMiddleware } from '@apollo/server/express4';
 import { ApolloServerPluginLandingPageLocalDefault } from '@apollo/server/plugin/landingPage/default';
-import { addMocksToSchema } from '@graphql-tools/mock';
 import { makeExecutableSchema } from '@graphql-tools/schema';
 import express from 'express';


addMocksToSchema was removed and there are no remaining references to @graphql-tools/mock in this package (it now only appears in package.json). Consider removing that devDependency to avoid carrying an unused dependency in builds.

Copilot · 2026-02-13T18:16:15Z

helm/sciserver/templates/graphql/graphql-deployment.yaml

          resources:
            {{- toYaml .Values.compute.resources | nindent 12 }}
          env:
+                    env:


There is a duplicated env: key here (env: followed by another env: with mismatched indentation). This will render invalid YAML for the Deployment and prevent the chart from installing. Remove the extra env: line and ensure the env var list is directly under the single env: key.

Suggested change

env:

Copilot · 2026-02-13T18:16:15Z

helm/sciserver/templates/graphql/graphql-deployment.yaml

            - name: FILES_BASE_URL
-              value: '{{ include "sciserver.url" . }}fileservice/api/'
+              value: '{{- if eq .Values.graphql.env "dev" -}}https://apps.sciserver.org/{{- else -}}{{ include "sciserver.url" . }}fileservice/api/{{- end -}}'
            - name: COMPUTE_BASE_URL
-              value: '{{ include "sciserver.url" . }}compute/api/'
+              value: '{{- if eq .Values.graphql.env "dev" -}}https://apps.sciserver.org/{{- else -}}{{ include "sciserver.url" . }}compute/api/{{- end -}}'
            - name: RACM_BASE_URL
-              value: '{{ include "sciserver.url" . }}racm/'
+              value: '{{- if eq .Values.graphql.env "dev" -}}https://apps.sciserver.org/{{- else -}}{{ include "sciserver.url" . }}racm/{{- end -}}'
            - name: LOGIN_PORTAL_BASE_URL
-              value: '{{ include "sciserver.url" . }}login-portal/api/'
+              value: '{{- if eq .Values.graphql.env "dev" -}}https://apps.sciserver.org/{{- else -}}{{ include "sciserver.url" . }}login-portal/api/{{- end -}}'


These base URLs no longer include the service path and (for FILES/COMPUTE/LOGIN_PORTAL) also drop the trailing / that the GraphQL REST data sources assume when they append paths like volumes/ or containers/. In dev this would call https://apps.sciserver.org/volumes/ instead of something like .../fileservice/api/volumes/, and in non-dev it risks producing .../apivolumes/. Update the conditional values so both branches include the correct service prefix and a trailing slash.

…onsistency

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-13T21:12:20Z

helm/sciserver/templates/web/web-deployment.yaml

              value: '{{ include "sciserver.url" . }}login-portal/'
            - name: NEXT_PUBLIC_GRAPHQL_URL
-              value: '{{ include "sciserver.url" . }}graphql'
+              value: '{{- if eq .Values.graphql.env "dev" -}}https://apps.sciserver.org/{{- else -}}{{ include "sciserver.url" . }}{{- end -}}graphql'


The PR description mentions updating graphql-deployment.yaml to set service base URLs conditionally based on the environment, but these changes are not present in the diff. The graphql-deployment.yaml file still uses non-conditional values for FILES_BASE_URL, COMPUTE_BASE_URL, RACM_BASE_URL, and LOGIN_PORTAL_BASE_URL. Please ensure that either the changes to graphql-deployment.yaml are included in this PR, or update the PR description to accurately reflect what changes are being made.

Copilot · 2026-02-13T21:12:21Z

helm/sciserver/templates/web/web-deployment.yaml

              value: '{{ include "sciserver.url" . }}login-portal/'
            - name: NEXT_PUBLIC_GRAPHQL_URL
-              value: '{{ include "sciserver.url" . }}graphql'
+              value: '{{- if eq .Values.graphql.env "dev" -}}https://apps.sciserver.org/{{- else -}}{{ include "sciserver.url" . }}{{- end -}}graphql'


When graphql.env is set to "dev", the GraphQL URL points to the production environment (https://apps.sciserver.org/). This means the dev web deployment will be making requests to the production GraphQL endpoint. Please confirm this is intentional. If the dev environment should use its own GraphQL service, this conditional logic may be inverted. Additionally, consider the implications of having dev clients communicate with production services, such as potential data consistency issues or unintended side effects on production data.

Suggested change

value: '{{- if eq .Values.graphql.env "dev" -}}https://apps.sciserver.org/{{- else -}}{{ include "sciserver.url" . }}{{- end -}}graphql'

value: '{{- if eq .Values.graphql.env "prod" -}}https://apps.sciserver.org/{{- else -}}{{ include "sciserver.url" . }}{{- end -}}graphql'

Copilot · 2026-02-13T21:12:21Z

components/ui/graphql/src/main.ts

-      mocks: { DateTime: () => new Date() }
-    }
-  ) : schema,
+  schema,


The removal of mocking logic has eliminated the DateTime mock resolver without providing a real resolver. The schema defines a DateTime scalar type, and it's used throughout the schema (e.g., Container.createdAt, Job.submissionTime, File.lastModified), but there's no resolver for it in the resolvers object. The mock previously provided DateTime: () => new Date(), which has now been removed. You need to add a DateTime resolver to handle this scalar type. Consider using DateTimeResolver from the graphql-scalars package (which is already used for JSONObject and URL), or implement a custom resolver that properly serializes, parses, and validates DateTime values.

Copilot · 2026-02-13T21:12:21Z

components/ui/graphql/src/main.ts

 import { expressMiddleware } from '@apollo/server/express4';
 import { ApolloServerPluginLandingPageLocalDefault } from '@apollo/server/plugin/landingPage/default';
-import { addMocksToSchema } from '@graphql-tools/mock';
 import { makeExecutableSchema } from '@graphql-tools/schema';


The import for makeExecutableSchema from @graphql-tools/schema is still present, but since addMocksToSchema has been removed, this package may no longer be necessary. Please verify if makeExecutableSchema is actually needed or if you can use the schema directly. If it's not required, consider removing both the import and the package from devDependencies in package.json to reduce bundle size and dependencies.

Refactor GraphQL server schema handling and update environment variab…

f59c3ae

…le URLs for deployment

peppermint-juli requested review from amitschang and Copilot February 13, 2026 18:12

Copilot started reviewing on behalf of peppermint-juli February 13, 2026 18:13 View session

Copilot AI reviewed Feb 13, 2026

View reviewed changes

peppermint-juli added 2 commits February 13, 2026 14:24

Fix indentation error in GraphQL deployment environment variables

76162e7

Update GraphQL environment variable URLs to fix formatting issues

37d427c

peppermint-juli marked this pull request as draft February 13, 2026 20:12

Update environment variable URLs in GraphQL and web deployments for c…

f6203af

…onsistency

peppermint-juli requested a review from Copilot February 13, 2026 21:08

peppermint-juli marked this pull request as ready for review February 13, 2026 21:08

Copilot started reviewing on behalf of peppermint-juli February 13, 2026 21:08 View session

Copilot AI reviewed Feb 13, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make graphql urls conditional#89

Make graphql urls conditional#89
peppermint-juli wants to merge 4 commits intomainfrom
point-prod

peppermint-juli commented Feb 13, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 13, 2026

Uh oh!

Copilot AI Feb 13, 2026

Uh oh!

Copilot AI Feb 13, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 13, 2026

Uh oh!

Copilot AI Feb 13, 2026

Uh oh!

Copilot AI Feb 13, 2026

Uh oh!

Copilot AI Feb 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

	value: '{{- if eq .Values.graphql.env "dev" -}}https://apps.sciserver.org/{{- else -}}{{ include "sciserver.url" . }}{{- end -}}graphql'
	value: '{{- if eq .Values.graphql.env "prod" -}}https://apps.sciserver.org/{{- else -}}{{ include "sciserver.url" . }}{{- end -}}graphql'

Conversation

peppermint-juli commented Feb 13, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant