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

oSPARC-S4L M2 ready #2555

Merged
merged 32 commits into from
Oct 2, 2021
Merged

Conversation

odeimaiz
Copy link
Member

@odeimaiz odeimaiz commented Sep 27, 2021

What do these changes do?

Milestone 2 ready:

  • No data tab for s4l product
  • App mode concept implemented:
    • No workbench, No logger, only full-screen
    • In dynamic services: Header and settings are hidden
    • If the pipeline is linear, App mode can be started and, by default, the entire pipeline will be part of the slideshow
  • Slideshow mode is persistent and an icon is shown in the study/template card
  • If a template is published in Guided mode/ App mode, the instance will be started in that mode
  • Make guided mode less buggy

Guided mode vs App mode:
Slideshow_guided-vs-app

Persistent and icons:
Slideshow_icons

App mode template:
Slideshow_template_app

Add nodes from breadcrumbs:
Slideshow_addNodes

Related issue/s

related to ITISFoundation/osparc-issues#496
closes #2561
Disable share link for templates #2560

How to test

Checklist

@odeimaiz odeimaiz added this to the Capra delle nevi milestone Sep 27, 2021
@odeimaiz odeimaiz self-assigned this Sep 27, 2021
@codecov
Copy link

codecov bot commented Sep 27, 2021

Codecov Report

Merging #2555 (7a19dd6) into master (fa61db5) will increase coverage by 0.0%.
The diff coverage is 0.0%.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #2555   +/-   ##
======================================
  Coverage    76.8%   76.8%           
======================================
  Files         623     623           
  Lines       24048   24050    +2     
  Branches     2356    2357    +1     
======================================
+ Hits        18488   18494    +6     
+ Misses       4932    4929    -3     
+ Partials      628     627    -1     
Flag Coverage Δ
integrationtests 65.4% <0.0%> (-0.1%) ⬇️
unittests 71.4% <0.0%> (+<0.1%) ⬆️

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

Impacted Files Coverage Δ
...mcore_service_webserver/projects/projects_utils.py 79.6% <0.0%> (-1.5%) ⬇️
...ce_webserver/resource_manager/garbage_collector.py 73.1% <0.0%> (-0.9%) ⬇️
.../director/src/simcore_service_director/producer.py 62.3% <0.0%> (+0.8%) ⬆️
...simcore_service_director_v2/modules/dask_client.py 92.5% <0.0%> (+1.6%) ⬆️
...webserver/computation_comp_tasks_listening_task.py 85.7% <0.0%> (+2.0%) ⬆️

@odeimaiz odeimaiz added the a:frontend issue affecting the front-end (area group) label Sep 27, 2021
@odeimaiz odeimaiz marked this pull request as ready for review September 28, 2021 12:06
@odeimaiz odeimaiz changed the title WIP: oSPARC-S4L M2 oSPARC-S4L M2 ready Sep 28, 2021
Copy link
Member

@pcrespov pcrespov left a comment

Choose a reason for hiding this comment

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

👍

Copy link
Contributor

@KZzizzle KZzizzle left a comment

Choose a reason for hiding this comment

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

looks all good to me - should it also have StS's review?
Just two comments:

  • I'm not sure it makes sense to allow making a node invisible through the "quick edit" since none of the other operations are possible (e.g. making a node visible, rearranging nodes) so it might be confusing to only that one function available. However if the PO's wanted it like that, then ignore my comment =)
  • When a new node is created via the guided mode "quick edit" and ports cannot be auto-connected perhaps there should be a warning?

@odeimaiz
Copy link
Member Author

  • I'm not sure it makes sense to allow making a node invisible through the "quick edit" since none of the other operations are possible (e.g. making a node visible, rearranging nodes) so it might be confusing to only that one function available. However if the PO's wanted it like that, then ignore my comment =)

When you start the App Mode all nodes are added to the breadcrumbs, the App/GM mode designer might want to keep a computational service hidden (it will be run when the user "steps over")

@ignapas
Copy link
Contributor

ignapas commented Oct 1, 2021

review in progress...

Copy link
Contributor

@ignapas ignapas left a comment

Choose a reason for hiding this comment

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

imo indicator icons in dashboard items should have some sort of explanatory tooltip

@odeimaiz odeimaiz merged commit cbaaeb2 into ITISFoundation:master Oct 2, 2021
@odeimaiz odeimaiz deleted the feature/osparc-s4l-m2 branch April 21, 2022 11:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:frontend issue affecting the front-end (area group)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Share Guided Mode Settings in Template
4 participants