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(client): add SSH support on the UI #2376

Merged
merged 3 commits into from
Feb 28, 2023
Merged

feat(client): add SSH support on the UI #2376

merged 3 commits into from
Feb 28, 2023

Conversation

lorenzo-cavazzi
Copy link
Member

@lorenzo-cavazzi lorenzo-cavazzi commented Feb 15, 2023 •
edited
Loading

Add SSH support in the UI, showing a "Connect to session with SSH" entry on the "start/connect" dropdown button on the dashboard and on the project header.

/deploy #persist #cypress renku=renku-ui-3.3.0 renku-notebooks=master extra-values=notebooks.ssh.enabled=true

fix #2324

@lorenzo-cavazzi lorenzo-cavazzi temporarily deployed to renku-ci-ui-2376 February 15, 2023 10:32 — with GitHub Actions Inactive
@RenkuBot
Copy link
Contributor

You can access the deployment of this PR at https://renku-ci-ui-2376.dev.renku.ch

@lorenzo-cavazzi lorenzo-cavazzi temporarily deployed to renku-ci-ui-2376 February 15, 2023 16:55 — with GitHub Actions Inactive
@lorenzo-cavazzi lorenzo-cavazzi temporarily deployed to renku-ci-ui-2376 February 15, 2023 18:14 — with GitHub Actions Inactive
@lorenzo-cavazzi lorenzo-cavazzi temporarily deployed to renku-ci-ui-2376 February 16, 2023 12:33 — with GitHub Actions Inactive
@lorenzo-cavazzi lorenzo-cavazzi temporarily deployed to renku-ci-ui-2376 February 17, 2023 10:51 — with GitHub Actions Inactive
@lorenzo-cavazzi lorenzo-cavazzi temporarily deployed to renku-ci-ui-2376 February 22, 2023 16:48 — with GitHub Actions Inactive
@lorenzo-cavazzi lorenzo-cavazzi temporarily deployed to renku-ci-ui-2376 February 23, 2023 08:53 — with GitHub Actions Inactive
@lorenzo-cavazzi lorenzo-cavazzi marked this pull request as ready for review February 24, 2023 13:50
@lorenzo-cavazzi lorenzo-cavazzi requested a review from a team as a code owner February 24, 2023 13:50
@lorenzo-cavazzi lorenzo-cavazzi temporarily deployed to renku-ci-ui-2376 February 24, 2023 13:50 — with GitHub Actions Inactive
@lorenzo-cavazzi lorenzo-cavazzi temporarily deployed to renku-ci-ui-2376 February 25, 2023 00:05 — with GitHub Actions Inactive
@lorenzo-cavazzi lorenzo-cavazzi temporarily deployed to renku-ci-ui-2376 February 27, 2023 08:24 — with GitHub Actions Inactive
@lorenzo-cavazzi lorenzo-cavazzi temporarily deployed to renku-ci-ui-2376 February 27, 2023 09:53 — with GitHub Actions Inactive
ciyer
ciyer requested changes Feb 27, 2023
View reviewed changes
Copy link
Contributor

@ciyer ciyer left a comment

Choose a reason for hiding this comment

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

Still working on testing the UI, but I have reviewed the code. The main thing I noticed is that the reducers can be simplified. Otherwise, it looks good!

ciyer
ciyer reviewed Feb 27, 2023
View reviewed changes
client/src/utils/components/ssh/ssh.tsx Outdated
Comment on lines 178 to 179
<img src={rkIconSshTicked} className="rk-icon rk-icon-md filter-green me-2" />
Your project supports SSH.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this text here is very small / easy to overlook.

image

I think we should make it a bit more prominent and set it off from the rest of the body text somehow.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point, I made it bold. Let me know if it works well with the new content.

@lorenzo-cavazzi lorenzo-cavazzi temporarily deployed to renku-ci-ui-2376 February 27, 2023 13:48 — with GitHub Actions Inactive
@lorenzo-cavazzi lorenzo-cavazzi temporarily deployed to renku-ci-ui-2376 February 27, 2023 16:45 — with GitHub Actions Inactive
@lorenzo-cavazzi
Copy link
Member Author

Thank you for the review @ciyer 🙏
I addressed all your points (mind that it takes ~40 mins to re-deploy due to referencing custom branches for renku-core and renku-notebooks).

Please double-check the changes in the text for the SSH instructions. Inspired by your and other offline comments, I ended up adding more information.

@lorenzo-cavazzi lorenzo-cavazzi temporarily deployed to renku-ci-ui-2376 February 28, 2023 07:32 — with GitHub Actions Inactive
@ciyer ciyer temporarily deployed to renku-ci-ui-2376 February 28, 2023 08:50 — with GitHub Actions Inactive
ciyer
ciyer approved these changes Feb 28, 2023
View reviewed changes
Copy link
Contributor

@ciyer ciyer left a comment

Choose a reason for hiding this comment

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

This looks great! 🎉 As discussed in the stand up, I added a small commit with minor cosmetic changes for visual consistency.

@lorenzo-cavazzi lorenzo-cavazzi temporarily deployed to renku-ci-ui-2376 February 28, 2023 10:34 — with GitHub Actions Inactive
@lorenzo-cavazzi lorenzo-cavazzi temporarily deployed to renku-ci-ui-2376 February 28, 2023 12:24 — with GitHub Actions Inactive
@lorenzo-cavazzi lorenzo-cavazzi temporarily deployed to renku-ci-ui-2376 February 28, 2023 13:47 — with GitHub Actions Inactive
@lorenzo-cavazzi lorenzo-cavazzi temporarily deployed to renku-ci-ui-2376 February 28, 2023 13:52 — with GitHub Actions Inactive
@lorenzo-cavazzi lorenzo-cavazzi mentioned this pull request Feb 28, 2023
Closed
@lorenzo-cavazzi lorenzo-cavazzi deployed to renku-ci-ui-2376 February 28, 2023 14:42 — with GitHub Actions Active
@lorenzo-cavazzi lorenzo-cavazzi merged commit 3ba8092 into master Feb 28, 2023
@lorenzo-cavazzi lorenzo-cavazzi deleted the 2324-ssh-ui branch February 28, 2023 14:54
@RenkuBot
Copy link
Contributor

Tearing down the temporary RenkuLab deplyoment for this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

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

🔶 SSH: verify ssh support on project pages
4 participants
@lorenzo-cavazzi @RenkuBot @ciyer @cramakri