Skip to content
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

feat: send js-ceramic and ceramic-one versions to CAS when creating requests #3246

Merged
merged 1 commit into from
Jun 24, 2024

Conversation

stbrody
Copy link
Contributor

@stbrody stbrody commented Jun 21, 2024

js-ceramic half corresponding to ceramicnetwork/ceramic-anchor-service#1232

Builds on #3245

@stbrody stbrody self-assigned this Jun 21, 2024
Copy link

linear bot commented Jun 21, 2024

@stbrody stbrody changed the base branch from develop to version-metric-aes-162 June 21, 2024 19:29
@stbrody stbrody changed the title feat: Publish js-ceramic and C1 versions as a metric once per hour feat: send js-ceramic and ceramic-one versions to CAS when creating requests Jun 21, 2024
Metrics.observe(VERSION_INFO, 1, {
jsCeramicVersion: this._versionInfo.cliPackageVersion,
jsCeramicGitHash: this._versionInfo.gitHash,
ceramicOneVersion: ipfsVersion,
ceramicOneVersion: this._versionInfo.ceramicOneVersion,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a small downside of this is that if ipfs/ceramic-one is upgraded out from under a running js-ceramic node, the js-ceramic node will continue to report the old version of ipfs/C1 until js-ceramic restarts. Since it should be rare to upgrade C1 without also at least restarting js-ceramic, I think this is acceptable.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah i think thats a really weird edge case, and we can also tell when it happens. i think this metric is fine

@stbrody stbrody requested review from gvelez17 and smrz2001 June 21, 2024 19:33
stateStoreDirectory,
anchorOnRequest: false,
indexing: {
db: `sqlite://${stateStoreDirectory}/ceramic.sqlite`,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is sqlite hardcoded in? just a side question

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a test file, so yes this test always uses sqlite

disableComposedb: true,
enableHistoricalSync: false,
},
pubsubTopic: '/ceramic/inmemory/test', // necessary so Ceramic instances can talk to each other
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would like to see a constant for this

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is just a test file, the value used here could be anything

Copy link
Contributor

@gvelez17 gvelez17 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

had a small change request but not critical, approved

Base automatically changed from version-metric-aes-162 to develop June 21, 2024 19:56
Copy link
Contributor

@smrz2001 smrz2001 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@stbrody stbrody force-pushed the send-version-to-cas-aes-133 branch from c930424 to 83d94cb Compare June 24, 2024 16:01
@stbrody stbrody enabled auto-merge (squash) June 24, 2024 16:02
@stbrody stbrody merged commit bd90887 into develop Jun 24, 2024
7 checks passed
@stbrody stbrody deleted the send-version-to-cas-aes-133 branch June 24, 2024 17:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants