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

Support [Matrix] summary endpoint #10782

Merged
merged 9 commits into from
Jan 3, 2025
Merged

Conversation

cyb3rko
Copy link
Contributor

@cyb3rko cyb3rko commented Jan 1, 2025

Refs #10776

Implements support for matrix-org/matrix-spec-proposals#3266.
Can be configured with the new parameter fetchMode and is enforced for matrix.org rooms.

Combined with #10778 (increases cache duration by a factor of 480), the amount of requests is reduced by a factor of 1440!

The performance is much better than the current implementation (few test runs as an example):

Copy link
Contributor

github-actions bot commented Jan 1, 2025

Messages
📖 ✨ Thanks for your contribution to Shields, @cyb3rko!

Generated by 🚫 dangerJS against 68d5224

Copy link
Member

@chris48s chris48s left a comment

Choose a reason for hiding this comment

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

Thanks for working on this.
I've left a few comments but I think this draft is in pretty good shape.

services/matrix/matrix.service.js Show resolved Hide resolved
services/matrix/matrix.service.js Outdated Show resolved Hide resolved
services/matrix/matrix.tester.js Outdated Show resolved Hide resolved
services/matrix/matrix.tester.js Outdated Show resolved Hide resolved
services/matrix/matrix.tester.js Show resolved Hide resolved
@chris48s chris48s added the service-badge New or updated service badge label Jan 2, 2025
Copy link
Member

@chris48s chris48s left a comment

Choose a reason for hiding this comment

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

nailed it - thanks

@chris48s chris48s added this pull request to the merge queue Jan 3, 2025
@chris48s
Copy link
Member

chris48s commented Jan 3, 2025

I'm going to deploy this shortly. I'm not sure if making the switch will be sufficient to get us unbanned from matrix.org or if we'll still be getting rejected on the summary endpoint as well? We'll have to wait and see what happens (which will take a while given we now have to wait ~4 hours for the existing badges to drop out of cache).

Merged via the queue into badges:master with commit 2c65301 Jan 3, 2025
23 checks passed
@cyb3rko cyb3rko deleted the matrix-summary branch January 3, 2025 10:33
@chris48s
Copy link
Member

chris48s commented Jan 3, 2025

Having done a quick test, I think the block only applies to guest account registration. These should start working again once the existing images expire from cache.


const queryParamSchema = Joi.object({
server_fqdn: Joi.string().hostname(),
fetchMode: Joi.string()

Choose a reason for hiding this comment

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

thanks for fixing this!
any chance we can still make this snake_case without breaking the world, though? 🙈

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All the base attributes for shields badges use camel case.
'server_fqdn' should be camel case as well but that would be a breaking change, so that's how we ended up here ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
service-badge New or updated service badge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants