Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Hack to add push priority to push notifications #7765

Merged
merged 8 commits into from
Jul 1, 2020
Merged

Hack to add push priority to push notifications #7765

merged 8 commits into from
Jul 1, 2020

Conversation

reivilibre
Copy link
Contributor

@reivilibre reivilibre commented Jul 1, 2020 •
edited
Loading

I think we'd like to properly solve this at some point but that's a Hard Problem™.

Pull Request Checklist

  • Pull request is based on the develop branch
  • Pull request includes a changelog file. The entry should:
    • Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from EventStore to EventWorkerStore.".
    • Use markdown where necessary, mostly for code blocks.
    • End with either a period (.) or an exclamation mark (!).
    • Start with a capital letter.
  • Pull request includes a sign off
  • Code style is correct (run the linters)

This corresponds to matrix-org/sygnal#69 (comment).

reivilibre added 4 commits July 1, 2020 09:33
@reivilibre
Remove obsolete comment about ancient temporary code
cddfa82 
Signed-off-by: Olivier Wilkinson (reivilibre) <olivier@librepush.net>
@reivilibre
Implement hack to set push priority
a2b3b88 
based on whether the tweaks indicate the event might cause
effects.
@reivilibre
Changelog for 7765
0965691 
Signed-off-by: Olivier Wilkinson (reivilibre) <olivier@librepush.net>
@reivilibre
Antilint
72cced2
@reivilibre reivilibre requested a review from a team July 1, 2020 09:17
babolivier
babolivier approved these changes Jul 1, 2020
View reviewed changes
Copy link
Contributor

@babolivier babolivier left a comment

Choose a reason for hiding this comment

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

lgtm. It'd be nice if there was a test for this change, but I'm not going to block merging the PR on this.
(also please remember to squash before merging)

synapse/push/httppusher.py Outdated Show resolved Hide resolved
@reivilibre reivilibre mentioned this pull request Jul 1, 2020
Merged
4 tasks
reivilibre and others added 3 commits July 1, 2020 14:36
@reivilibre
Add tests for push priority
62f3373 
Signed-off-by: Olivier Wilkinson (reivilibre) <olivier@librepush.net>
@reivilibre @babolivier
Update synapse/push/httppusher.py
833e69d 
Co-authored-by: Brendan Abolivier <babolivier@matrix.org>
@reivilibre
Antilint
ad082ff
@reivilibre reivilibre requested a review from babolivier July 1, 2020 14:08
babolivier
babolivier approved these changes Jul 1, 2020
View reviewed changes
Copy link
Contributor

@babolivier babolivier left a comment

Choose a reason for hiding this comment

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

lgtm! Thanks for taking the time to write the tests :)

tests/push/test_http.py Outdated Show resolved Hide resolved
@reivilibre reivilibre requested a review from babolivier July 1, 2020 15:53
@reivilibre reivilibre merged commit e5808c4 into develop Jul 1, 2020
@reivilibre reivilibre deleted the rei/pushpriority branch July 1, 2020 16:02
@reivilibre reivilibre mentioned this pull request Jul 2, 2020
Closed
babolivier pushed a commit that referenced this pull request Sep 1, 2021
@anoadragon453
Merge commit '5cdca53aa' into dinsic
60a96e4 
* commit '5cdca53aa':
  Merge different Resource implementation classes (#7732)
  Fix inconsistent handling of upper and lower cases of email addresses. (#7021)
  Allow YAML config file to contain None (#7779)
  Fix a typo.
  Move 1.15.2 after 1.16.0rc2.
  1.16.0rc2
  Remove an extraneous space.
  Add links to the fixes.
  Fix tense in the release notes.
  Hack to add push priority to push notifications (#7765)
  Add early returns to `_check_for_soft_fail` (#7769)
  Use symbolic names for replication stream names (#7768)
  Type checking for `FederationHandler` (#7770)
  Fix new metric where we used ms instead of seconds (#7771)
  Fix incorrect error message when database CTYPE was set incorrectly. (#7760)
  Pin link in CHANGES.md
  Fixes to CHANGES.md
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@babolivier babolivier babolivier approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@reivilibre @babolivier