-
Notifications
You must be signed in to change notification settings - Fork 3
Report version from manifest #129
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
Changes from 8 commits
ffde8e0
cbee166
93eba67
1f4a528
4ad9ee8
c8a9209
5239115
9a84046
ede943f
6a40a04
7e432ec
740ec9b
912d857
3e84548
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 |
|---|---|---|
|
|
@@ -31,9 +31,20 @@ import { | |
| type WSS | ||
| } from './pubsub.ts' | ||
| import { addChannelToSubscription, deleteChannelFromSubscription, postEvent, pushServerActionhandlers, subscriptionInfoWrapper } from './push.ts' | ||
| import { pathToFileURL } from 'node:url' | ||
| // @deno-types="npm:@types/nconf" | ||
| import nconf from 'npm:nconf' | ||
|
|
||
| const cheloniaAppManifest = await (async () => { | ||
| try { | ||
| return (await import(pathToFileURL(join(process.cwd(), 'chelonia.json')).toString(), { | ||
|
corrideat marked this conversation as resolved.
Outdated
|
||
| with: { type: 'json' } | ||
| })).default | ||
| } catch { | ||
| console.warn('`chelonia.json` unparsable or not found. Version information will be unavailable.') | ||
| } | ||
| })() | ||
|
|
||
| const ARCHIVE_MODE = nconf.get('server:archiveMode') | ||
|
|
||
| if (CREDITS_WORKER_TASK_TIME_INTERVAL && OWNER_SIZE_TOTAL_WORKER_TASK_TIME_INTERVAL > CREDITS_WORKER_TASK_TIME_INTERVAL) { | ||
|
|
@@ -49,8 +60,6 @@ const creditsWorker = ARCHIVE_MODE || !CREDITS_WORKER_TASK_TIME_INTERVAL | |
| ? undefined | ||
| : createWorker(join(import.meta.dirname || '.', import.meta.workerDir || '.', 'creditsWorker.js')) | ||
|
|
||
| const { CONTRACTS_VERSION, GI_VERSION } = process.env | ||
|
|
||
| // Dynamic runtime import to bypass bundling issues with npm: specifier | ||
| const hapi = new Hapi.Server({ | ||
| // debug: false, // <- Hapi v16 was outputing too many unnecessary debug statements | ||
|
|
@@ -426,8 +435,11 @@ sbp('okTurtles.data/set', PUBSUB_INSTANCE, createServer(hapi.listener, { | |
| serverHandlers: { | ||
| connection (socket) { | ||
| const versionInfo = { | ||
| GI_VERSION: GI_VERSION || null, | ||
| CONTRACTS_VERSION: CONTRACTS_VERSION || null | ||
| appVersion: cheloniaAppManifest?.appVersion || null, | ||
| contractsVersion: cheloniaAppManifest?.contracts ? Object.fromEntries( | ||
| Object.entries(cheloniaAppManifest?.contracts) | ||
| .map(([k, v]) => [k, (v as { version: string }).version]) | ||
| ) : null | ||
|
Comment on lines
+449
to
+453
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Question: does this design assume there will only be one app per server? If so, that's clearly not a safe assumption (per the other open issues). So I think we should address this somehow in this PR in a way that either fully addresses it, or sets up future versions to address it properly. I.e.:
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. At the moment there can only possibly be one app per server. Since this is being sent when a WS connection is established, I think this is fine (since presumably, a WS client will be for a single app). I think it's low risk to make this assumption since, whichever changes are needed, they are limited to the server API and will not require changing contracts. In addition, presumably, we'll want With this in mind, these changes look safe and perhaps even future proof:
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Currently, yes. But, I think little if anything will change from the client's perspective. |
||
| } | ||
| socket.send(createNotification(NOTIFICATION_TYPE.VERSION_INFO, versionInfo)) | ||
| } | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.