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

351 #697

Draft
wants to merge 13 commits into
base: master
Choose a base branch
from
Draft

351 #697

wants to merge 13 commits into from

Conversation

oliver-sanders
Copy link
Member

@oliver-sanders oliver-sanders commented Jun 30, 2021

built on #543
closes #351

Developed for reviewing #543, and monitoring the performance of the workflowService, shows that active subscriptions and subscribers.

Ugly but effective:

Screenshot 2021-06-30 at 16 00 52

Requirements check-list

  • I have read CONTRIBUTING.md and added my name as a Code Contributor.
  • Contains logically grouped changes (else tidy your branch by rebase).
  • Does not contain off-topic changes (use other PRs for other changes).
  • Appropriate tests are included (unit and/or functional).
  • Appropriate change log entry included.
  • No documentation update required.

kinow and others added 13 commits June 25, 2021 11:13
…in the Workflow Service.

This way we can have multiple components and views re-using the same queries
that are merged. Or they can use different queries.

When different queries are used (with different names, of course), the subscriptions
for these queries are kept apart. This is useful when you have, for instance, a subscription
like GScan's that has no variables, while Tree's query uses variables and different
structure.

The lifecycle of subscriptions is not maintained  by the Workflow Service. It is now maintained
via lifecycle methods of components and navigation guards of views. The code for that is in
mixins.

After these changes, the Workflow view can now display any View or Component that uses Subscriptions.
And the Workflow view does not have to coordinate when widgets are created with their subscriptions.

Unit tests and e2e tests will have to be updated, and new tests added as well. But with these
changes, the Table view PR should be more easily merged. Besides fixing tests, we will also need
to sort out the query merge that appears to have a few cases where queries are not merged
correctly.
…, and dismissed (and removed from central data store)
@oliver-sanders oliver-sanders added this to the 1.0 milestone Jun 30, 2021
@oliver-sanders oliver-sanders self-assigned this Jun 30, 2021
@codecov-commenter
Copy link

codecov-commenter commented Jun 30, 2021

Codecov Report

Merging #697 (b3bb2cf) into master (7783bd7) will decrease coverage by 0.02%.
The diff coverage is 92.23%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #697      +/-   ##
==========================================
- Coverage   88.65%   88.63%   -0.03%     
==========================================
  Files          69       80      +11     
  Lines        1499     1566      +67     
  Branches      117      114       -3     
==========================================
+ Hits         1329     1388      +59     
- Misses        135      146      +11     
+ Partials       35       32       -3     
Flag Coverage Δ
e2e 76.75% <67.57%> (-0.10%) ⬇️
unittests 79.89% <95.56%> (+3.20%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/components/cylc/Drawer.vue 75.00% <ø> (ø)
src/components/cylc/Subscriptions.vue 0.00% <0.00%> (ø)
src/components/cylc/tree/Tree.vue 78.78% <ø> (ø)
src/components/cylc/tree/nodes.js 100.00% <ø> (ø)
src/components/cylc/workflow/Toolbar.vue 77.77% <ø> (ø)
src/graphql/index.js 100.00% <ø> (ø)
src/layouts/Default.vue 66.66% <ø> (ø)
src/main.js 100.00% <ø> (ø)
src/mixins/graphql.js 100.00% <ø> (ø)
src/mixins/index.js 100.00% <ø> (ø)
... and 44 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7783bd7...b3bb2cf. Read the comment docs.

@kinow
Copy link
Member

kinow commented Jun 30, 2021

Ha! Had a post-it with a similar idea! But showing a small window at the bottom of the page and only in development mode. I like the idea to use a widget for that. Much easier 👍

@oliver-sanders
Copy link
Member Author

TODO: the data is not currently reactive

@hjoliver
Copy link
Member

hjoliver commented Jul 5, 2021

This will be good to have 👍

@kinow kinow modified the milestones: 1.0, 2.0 Sep 10, 2021
@oliver-sanders oliver-sanders modified the milestones: 2.0.0, 1.x Feb 16, 2022
@oliver-sanders oliver-sanders modified the milestones: 1.x, Pending Jun 8, 2022
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.

Subscription view
4 participants