feature/refactor-environment #339
Conversation
packages/core/App.tsx
Outdated
| @@ -39,15 +39,11 @@ interface AppProps { | |||
| // Localhost: "https://localhost:9081" | |||
| // Stage: "http://stg-aics-api.corp.alleninstitute.org" | |||
| // From the web (behind load balancer): "/" | |||
| aicsLoadBalancerBaseUrl?: string; | |||
| fileExplorerServiceBaseUrl?: string; | |||
| environment?: string; | |||
There was a problem hiding this comment.
You can use the enum as the type value here (Environment)
| environment?: string; | |
| environment?: Environment; |
There was a problem hiding this comment.
resolved in 07a2715
packages/core/constants/index.ts
Outdated
| STAGING = "http://stg-aics.corp.alleninstitute.org", | ||
| PRODUCTION = "http://aics.corp.alleninstitute.org", | ||
| } | ||
| export const Environment = { |
There was a problem hiding this comment.
This might be better as an enum
There was a problem hiding this comment.
resolved in 07a2715
| applicationVersion?: string; | ||
| baseUrl?: string | keyof typeof FileExplorerServiceBaseUrl; | ||
| fileExplorerServiceBaseUrl?: string | keyof typeof FESBaseUrlMap; |
There was a problem hiding this comment.
Ah I see it is used here. You can use an enum type here to avoid having to do this complicated typing once you convert FESBaseUrlMap to an enum
Ex.
| fileExplorerServiceBaseUrl?: string | keyof typeof FESBaseUrlMap; | |
| fileExplorerServiceBaseUrl?: FESEnvironmentSpecificBaseUrl; |
There was a problem hiding this comment.
resolved in 07a2715
| import ExecutionEnvServiceNoop from "../../services/ExecutionEnvService/ExecutionEnvServiceNoop"; | ||
| import { UserSelectedApplication } from "../../services/PersistentConfigService"; | ||
| import NotificationServiceNoop from "../../services/NotificationService/NotificationServiceNoop"; | ||
| import DatabaseServiceNoop from "../../services/DatabaseService/DatabaseServiceNoop"; | ||
| import PublicDataset from "../../../web/src/entity/PublicDataset"; | ||
|
|
||
| import { Environment } from "../../constants"; |
There was a problem hiding this comment.
Strange import, combine into above list
There was a problem hiding this comment.
resolved in 07a2715
| const fileService = new HttpFileService({ | ||
| baseUrl, | ||
| fileExplorerServiceBaseUrl: fileExplorerServiceBaseUrl, |
There was a problem hiding this comment.
if you want to be extra javascripty, this and line 189 could just be fileExplorerServiceBaseUrl,
There was a problem hiding this comment.
Is it bad to say I don't want to be? I kept moving around variables and stuff broke.
There was a problem hiding this comment.
fair enough! just noted since elsewhere in file sometimes (or used to?) use this pattern
|
I came across a bug where we save the selected data source (or at least what one is checked) in the desktop app but when we refresh the datasource returns to "Production". Now that we have write requests this is dangerous. In this PR I added a persistent 'Environment' as to align with the checked box. (even through a cache clear) |
BrianWhitneyAI
merged commit 390a804
into
feature/copy-to-local-cache-request
The purpose of this PR is to refactor the base urls that we are saving as global variables into one variable that can be mapped to various Urls. A few things occurred during this refactor
FileExplorerServiceBaseURlandAicsLoadBalancerBaseUrlin favor ofEnvironmentbaseUrltoFileExplorerServiceBaseURlFES,MMS, andLoadBalancertointeraction.selectors.ts