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

ui: add a way to open, list and close interactive sessions #142

Merged
merged 1 commit into from
Oct 22, 2020
Merged

ui: add a way to open, list and close interactive sessions #142

merged 1 commit into from
Oct 22, 2020

Conversation

audrium
Copy link
Member

@audrium audrium commented Oct 15, 2020

closes #77

@audrium audrium force-pushed the interactive_sessions_workflow_pages branch from 47be429 to c5cb8d5 Compare October 15, 2020 15:54
@audrium audrium changed the title WIP: ui: add way to open, list and close interactive sessions WIP: ui: add a way to open, list and close interactive sessions Oct 15, 2020
@audrium audrium force-pushed the interactive_sessions_workflow_pages branch from c5cb8d5 to bd8db90 Compare October 16, 2020 11:29
@audrium audrium changed the title WIP: ui: add a way to open, list and close interactive sessions ui: add a way to open, list and close interactive sessions Oct 16, 2020
@audrium audrium requested a review from mvidalgarcia October 16, 2020 11:39
@audrium
Copy link
Member Author

audrium commented Oct 16, 2020

Things to keep in mind while reviewing:

  • In order to reuse WorkflowDeleteModal and WorkflowActionsPopup (or simply the hamburger menu) components in Workflow Details page I had to refactor these to be controller from redux just to avoid holding all the state in parent components. This would have lead to lots of code duplication.
  • I've implemented a temporary solution to reload workflow list when user opens, closes interactive session or deletes the workflow. Left a comment about it here
  • I've noticed that if the request to fetch configs fails for some reason (e.g. user did not set his token), then pollingSecs becomes undefined which leads to an infinite loop of get workflows requests. Problem is fixed in this pr.
  • We currently don't have a way to fetch a single workflow which leads to some problems refreshing workflowDetails page after user does some changes (opens, closes interactive session or deletes the workflow). This could be solved in a separate issue. For now left a comment here

mvidalgarcia
mvidalgarcia reviewed Oct 19, 2020
View reviewed changes
reana-ui/src/actions.js Outdated Show resolved Hide resolved
reana-ui/src/components/JupyterNotebookIcon.js Outdated Show resolved Hide resolved
reana-ui/src/pages/workflowDetails/components/WorkflowInfo.js Outdated Show resolved Hide resolved
reana-ui/src/util.js Outdated
* Open Jupyter Notebook in a new tab.
*/
export const openJupyterNotebook = (event, sessionUri, reanaToken) => {
const uri = `${api}${sessionUri}?token=${reanaToken}`;
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to append the token? I think that if the session is valid the token shouldn't be needed.

Copy link
Member Author

Choose a reason for hiding this comment

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

If we do not append the token it opens up a login screen where token needs to be entered. We also append it in CLI. This is something to be fixed quite urgently I think

Copy link
Member

Choose a reason for hiding this comment

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

I can see what you mean now. It's not cool to expose the token like this when we're already logged in. Maybe we can create a ticket for this?

Copy link
Member

Choose a reason for hiding this comment

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

I agree, this is part of a different issue since it touches more broader topics.

Short explanation: This is due to login living inside REANA-Server (more exactly, inside Invenio). In order to achieve the described behaviour, we would need to:

  1. Place an authentication service in front of all of these components (R-Server and interactive sessions) (going more in the direction of having Kubernetes based authentications system).
  2. Or have an authentication mechanism that allows federated authentication

reana-ui/src/util.js Show resolved Hide resolved
reana-ui/src/util.js Outdated Show resolved Hide resolved
reana-ui/src/pages/workflowDetails/components/WorkflowInfo.js Outdated Show resolved Hide resolved
reana-ui/src/pages/workflowList/components/WorkflowActionsPopup.js Outdated Show resolved Hide resolved
reana-ui/src/pages/workflowList/components/WorkflowActionsPopup.js Outdated Show resolved Hide resolved
reana-ui/src/pages/workflowList/components/WorkflowList.js Outdated Show resolved Hide resolved
@audrium audrium mentioned this pull request Oct 21, 2020
Open
@audrium audrium force-pushed the interactive_sessions_workflow_pages branch 2 times, most recently from 9e4f183 to 6f37ddf Compare October 21, 2020 13:45
mvidalgarcia
mvidalgarcia reviewed Oct 21, 2020
View reviewed changes
Copy link
Member

@mvidalgarcia mvidalgarcia left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I see that deleted workflows can open/close sessions, should this be possible?

image

ccing @diegodelemos @tiborsimko

reana-ui/src/util.js Outdated Show resolved Hide resolved
reana-ui/src/pages/workflowDetails/WorkflowDetails.js Show resolved Hide resolved
@audrium audrium force-pushed the interactive_sessions_workflow_pages branch from 6f37ddf to c852961 Compare October 21, 2020 14:29
@mvidalgarcia mvidalgarcia mentioned this pull request Oct 21, 2020
Closed
@diegodelemos
Copy link
Member

I see that deleted workflows can open/close sessions, should this be possible?

This sounds very counterintuitive/confusing to me; although the interactive session has nothing to do with the workflow and its status, I would probably not show this action if the workflow is deleted. Once the workspace is disassociated from the workflow this would be displayed completely differently (I am thinking on the use case that someone uses the workflow as a workspace right now).

@audrium audrium force-pushed the interactive_sessions_workflow_pages branch from c852961 to 0246341 Compare October 21, 2020 15:12
@mvidalgarcia mvidalgarcia merged commit 0246341 into reanahub:master Oct 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@diegodelemos diegodelemos diegodelemos left review comments

@mvidalgarcia mvidalgarcia mvidalgarcia 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.

sessions: easy way to open, list and close interactive sessions
3 participants
@audrium @diegodelemos @mvidalgarcia