-
Notifications
You must be signed in to change notification settings - Fork 104
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
fix: add helia version to agent version #128
Changes from all commits
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 |
---|---|---|
@@ -0,0 +1,14 @@ | ||
import { readFile, writeFile } from 'fs/promises' | ||
|
||
const pkg = JSON.parse( | ||
await readFile( | ||
new URL('../package.json', import.meta.url) | ||
) | ||
) | ||
|
||
await writeFile( | ||
new URL('../src/version.ts', import.meta.url), | ||
`export const version = '${pkg.version}' | ||
export const name = '${pkg.name}' | ||
` | ||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
export const version = '0.0.0' | ||
export const name = 'helia' | ||
Comment on lines
+1
to
+2
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. why not have this file be a json file 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. Node 18 still shows a warning if you import json from js:
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,10 @@ | ||
/* eslint-env mocha */ | ||
|
||
import { webSockets } from '@libp2p/websockets' | ||
import { expect } from 'aegir/chai' | ||
import { Key } from 'interface-datastore' | ||
import { createLibp2p } from 'libp2p' | ||
import { identifyService } from 'libp2p/identify' | ||
import { CID } from 'multiformats/cid' | ||
import { createHelia } from '../src/index.js' | ||
import type { Helia } from '@helia/interface' | ||
|
@@ -35,4 +38,50 @@ describe('helia factory', () => { | |
await helia.datastore.put(key, block) | ||
expect(await helia.datastore.has(key)).to.be.true() | ||
}) | ||
|
||
it('adds helia details to the AgentVersion', async () => { | ||
helia = await createHelia() | ||
|
||
const peer = await helia.libp2p.peerStore.get(helia.libp2p.peerId) | ||
const agentVersionBuf = peer.metadata.get('AgentVersion') | ||
const agentVersion = new TextDecoder().decode(agentVersionBuf) | ||
Comment on lines
+46
to
+47
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. does it make sense to maintain and expose a method for obtaining a decoded agent version ? I can imagine having to decode this agent version being a minor pain point for users. 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. The AgentVersion is used by the identify protocol and it's not something people generally interact with directly. |
||
|
||
expect(agentVersion).to.include('helia/') | ||
}) | ||
|
||
it('does not add helia details to the AgentVersion when it has been overridden', async () => { | ||
helia = await createHelia({ | ||
libp2p: await createLibp2p({ | ||
transports: [ | ||
webSockets() | ||
], | ||
services: { | ||
identity: identifyService({ | ||
agentVersion: 'my custom agent version' | ||
}) | ||
} | ||
}) | ||
}) | ||
|
||
const peer = await helia.libp2p.peerStore.get(helia.libp2p.peerId) | ||
const agentVersionBuf = peer.metadata.get('AgentVersion') | ||
const agentVersion = new TextDecoder().decode(agentVersionBuf) | ||
|
||
expect(agentVersion).to.not.include('helia/') | ||
}) | ||
|
||
it('does not add helia details to the AgentVersion when identify is not configured', async () => { | ||
helia = await createHelia({ | ||
libp2p: await createLibp2p({ | ||
transports: [ | ||
webSockets() | ||
] | ||
}) | ||
}) | ||
|
||
const peer = await helia.libp2p.peerStore.get(helia.libp2p.peerId) | ||
const agentVersionBuf = peer.metadata.get('AgentVersion') | ||
|
||
expect(agentVersionBuf).to.be.undefined() | ||
}) | ||
}) |
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.
I'm not saying we need to open a new PR, but I think it would be nice in future for these kinds of things if we linked to the source of the information (presumably somewhere in js-libp2p).
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.
Agree, but it’s buried fairly deeply in the identify service so would need a change there - it’s not something that’s needed to be accessed before.