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

frontend: Add attach to pod option #1030

Merged
merged 1 commit into from
Apr 13, 2023
Merged

frontend: Add attach to pod option #1030

merged 1 commit into from
Apr 13, 2023

Conversation

ashu8912
Copy link
Member

@ashu8912 ashu8912 commented Mar 16, 2023 •
edited
Loading

Acceptance Criteria:

  • Go to pods detail view and there is a attach header action
  • Click on the attach header action and you are able to connect to the container stdin, stdout, stderr whichever is available
  • Change the container and you get attached to the new container i/o/err stream now

@ashu8912 ashu8912 requested review from joaquimrocha and illume and removed request for joaquimrocha March 16, 2023 13:09
joaquimrocha
joaquimrocha requested changes Mar 23, 2023
View reviewed changes
Copy link
Collaborator

@joaquimrocha joaquimrocha left a comment

Choose a reason for hiding this comment

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

Good job. I tried it and it works but needs some improvements in terms of UX and the code.
Also, maybe the terminal should have a title "Attach: POD_NAME" instead of "Terminal: POD_NAME"

frontend/src/components/common/Terminal.tsx Outdated Show resolved Hide resolved
frontend/src/components/common/Terminal.tsx Show resolved Hide resolved
frontend/src/components/common/Terminal.tsx Outdated Show resolved Hide resolved
frontend/src/components/common/Terminal.tsx Outdated Show resolved Hide resolved
frontend/src/components/common/Terminal.tsx Outdated Show resolved Hide resolved
frontend/src/components/common/Terminal.tsx Outdated Show resolved Hide resolved
frontend/src/components/common/Terminal.tsx Outdated Show resolved Hide resolved
frontend/src/components/common/Terminal.tsx Outdated Show resolved Hide resolved
frontend/src/components/common/Terminal.tsx Outdated Show resolved Hide resolved
frontend/src/components/common/Terminal.tsx Outdated Show resolved Hide resolved
joaquimrocha
joaquimrocha requested changes Mar 29, 2023
View reviewed changes
Copy link
Collaborator

@joaquimrocha joaquimrocha left a comment

Choose a reason for hiding this comment

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

Just a couple minor changes left.

frontend/src/components/common/Terminal.tsx Outdated Show resolved Hide resolved
frontend/src/components/common/Terminal.tsx Outdated Show resolved Hide resolved
joaquimrocha
joaquimrocha requested changes Apr 3, 2023
View reviewed changes
Copy link
Collaborator

@joaquimrocha joaquimrocha left a comment

Choose a reason for hiding this comment

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

Awesome!. Left one very small comment.

frontend/src/components/common/Terminal.tsx Outdated Show resolved Hide resolved
@ashu8912 @joaquimrocha
frontend: Add attach to pod
0e836ad 
We add a new button to the pod details page that allows the user to
attach to the pod's container(by default the first available one),
using the terminal component. We also update the terminal component
to accept a new prop called isAttach which allows us to differentiate
b/w a exec and a attach session.
@ashu8912 ashu8912 merged commit 5415d13 into main Apr 13, 2023
@illume illume deleted the add-attach-to-pod branch April 14, 2023 17:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joaquimrocha joaquimrocha joaquimrocha approved these changes

@illume illume Awaiting requested review from illume

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@ashu8912 @joaquimrocha