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.

Sign up for GitHub

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

Jump to bottom

feat: document publications and remove mongo queries from parameters SOFIE-1183 #1062

Merged
merged 30 commits into from
Nov 17, 2023
Merged

feat: document publications and remove mongo queries from parameters SOFIE-1183 #1062

merged 30 commits into from
Nov 17, 2023

Conversation

Julusian
Copy link
Member

@Julusian Julusian commented Oct 31, 2023 •
edited
Loading

  • What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

feature

builds on #1056

  • What is the current behavior? (You can also link to an open issue here)

The publications exposed over ddp are mostly done by allowing the caller to define the mongo query that will be used.

  • What is the new behavior (if this is a feature change)?

These have all been replaced to provide a strict set of parameters that can be provided in the publication call.
The aim of this is to make the code more self-documenting on what publications are being run, and make it easier for us to figure review what mongo queries the subscriptions are running, so that they can both be optimised (with indices) and combined where necessary

  • Other information:

No attempt has been made to optimise the usages of these publications (except a couple of very simple circumstances).

Further analysis could be done to look into:

  1. Is is better to run a subscription per rundown, or for an array of rundownIds? In particular, what is the network/cpu impact when the list of rundownIds changes?
  2. Are all of the subscriptions necessary? It seems like many deep inside of views are running subscriptions. Could/should they be pushed out to higher levels? Would there be any benefit to doing so considering how meteor deduplicates the content?

Status

  • Code documentation for the relevant parts in the code have been added/updated by the PR author
  • The functionality has been tested by the PR author
  • Automated tests to cover the new functionality and/or guard against regressions have been added
  • The functionality has been tested by NRK

@codecov Codecov
Copy link

codecov bot commented Oct 31, 2023 •
edited
Loading

Codecov Report

Attention: 931 lines in your changes are missing coverage. Please review.

Comparison is base (dfebb3f) 58.21% compared to head (fc5eed2) 58.13%.
Report is 19 commits behind head on release51.

Files Patch % Lines
meteor/server/publications/rundown.ts 0.00% 419 Missing ⚠️
meteor/server/publications/organization.ts 0.00% 66 Missing ⚠️
meteor/server/publications/showStyle.ts 0.00% 66 Missing ⚠️
meteor/server/publications/studio.ts 0.00% 57 Missing ⚠️
meteor/server/publications/buckets.ts 0.00% 50 Missing ⚠️
meteor/server/publications/peripheralDevice.ts 0.00% 49 Missing ⚠️
meteor/server/publications/rundownPlaylist.ts 0.00% 46 Missing ⚠️
meteor/server/publications/system.ts 0.00% 25 Missing ⚠️
meteor/server/publications/studioUI.ts 0.00% 21 Missing ⚠️
meteor/lib/collections/lib.ts 38.46% 16 Missing ⚠️
... and 28 more
Additional details and impacted files 
@@              Coverage Diff              @@
##           release51    #1062      +/-   ##
=============================================
- Coverage      58.21%   58.13%   -0.08%     
=============================================
  Files            507      507              
  Lines          81418    81824     +406     
  Branches        4282     4328      +46     
=============================================
+ Hits           47397    47569     +172     
- Misses         33963    34205     +242     
+ Partials          58       50       -8

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Julusian Julusian changed the title Feat/improve publications feat: document publications and remove mongo queries from parameters Oct 31, 2023
Base automatically changed from feat/typed-gateway-publications2 to release51 November 10, 2023 10:44
@Julusian Julusian force-pushed the feat/improve-publications branch from 02679e8 to b953121 Compare November 10, 2023 10:49
@Julusian Julusian force-pushed the feat/improve-publications branch from b953121 to e2523c3 Compare November 10, 2023 10:57
@Julusian Julusian marked this pull request as ready for review November 10, 2023 10:59
@Julusian Julusian changed the title feat: document publications and remove mongo queries from parameters feat: document publications and remove mongo queries from parameters SOFIE-1183 Nov 10, 2023
@Julusian Julusian requested a review from a team November 10, 2023 11:53
nytamin
nytamin approved these changes Nov 16, 2023
View reviewed changes
packages/corelib/src/pubsub.ts Show resolved Hide resolved
meteor/lib/api/pubsub.ts Show resolved Hide resolved
meteor/lib/api/pubsub.ts Show resolved Hide resolved
packages/corelib/src/pubsub.ts Outdated Show resolved Hide resolved
packages/corelib/src/pubsub.ts Outdated Show resolved Hide resolved
packages/corelib/src/pubsub.ts Show resolved Hide resolved
@Julusian Julusian merged commit 17bb2e3 into release51 Nov 17, 2023
64 of 66 checks passed
@Julusian Julusian deleted the feat/improve-publications branch November 17, 2023 13:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nytamin nytamin nytamin 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
@Julusian @nytamin