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/dashboard supports random port #874

Merged
merged 20 commits into from
Sep 30, 2022
Merged

Feat/dashboard supports random port #874

merged 20 commits into from
Sep 30, 2022

Conversation

imneov
Copy link
Contributor

@imneov imneov commented Jan 15, 2022 •
edited by mukundansundar
Loading

Description

  1. support dashboardLocalPort set to 0
  2. update pf.LocalPort when PortForward is Ready

Issue reference

Please reference the issue this PR will close: #863

Checklist

Please make sure you've completed the relevant tasks for this PR, out of the following list:

  • Code compiles correctly
  • Created/updated tests
  • Extended the documentation

@imneov imneov requested review from a team as code owners January 15, 2022 01:07
imneov added 3 commits January 15, 2022 13:16
@imneov
feat: dashboard --port flag support set to 0
07f9567 
Signed-off-by: 柳丰 <grantliu@yunify.com>
@imneov
feat: update readme
d90ab39 
Signed-off-by: 柳丰 <grantliu@yunify.com>
@imneov
feat: add error
f71c95f 
Signed-off-by: 柳丰 <grantliu@yunify.com>
@imneov imneov force-pushed the feat/dashboard-supports-random-port branch from d8e8000 to f71c95f Compare January 15, 2022 05:16
@mukundansundar
Copy link
Collaborator

@imneov Please fix linter errors ...

imneov and others added 2 commits March 7, 2022 10:14
mukundansundar
mukundansundar reviewed Mar 7, 2022
View reviewed changes
Copy link
Collaborator

@mukundansundar mukundansundar left a comment

Choose a reason for hiding this comment

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

@imneov Thanks for the contribution.
Overall looks good. Just some small changes.

@codecov
Copy link

codecov bot commented Mar 7, 2022 •
edited
Loading

Codecov Report

Merging #874 (8df3aec) into master (9e07fcd) will decrease coverage by 0.07%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master     #874      +/-   ##
==========================================
- Coverage   29.20%   29.13%   -0.08%     
==========================================
  Files          35       35              
  Lines        2359     2365       +6     
==========================================
  Hits          689      689              
- Misses       1594     1600       +6     
  Partials       76       76
Impacted Files Coverage Δ
pkg/kubernetes/portforward.go 0.00% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@imneov
Copy link
Contributor Author

imneov commented Mar 7, 2022

@mukundansundar Thanks for the suggestions.

@mukundansundar
Copy link
Collaborator

@imneov Please do fix the DCO and the conflicts ...

@imneov imneov force-pushed the feat/dashboard-supports-random-port branch from 575f5ee to b37fb1b Compare March 8, 2022 04:05
@imneov
Copy link
Contributor Author

imneov commented Mar 8, 2022

@imneov Please do fix the DCO and the conflicts ...

OK, It`s done.

@imneov
fix: code improve
028c0b9 
Signed-off-by: imneov <grantliu@yunify.com>
mukundansundar
mukundansundar reviewed Mar 9, 2022
View reviewed changes
Copy link
Collaborator

@mukundansundar mukundansundar left a comment

Choose a reason for hiding this comment

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

@imneov Almost done ... just a few small changes and linter fixes ...

@imneov
fix: small changes and linter fixes
67a8ea7 
Signed-off-by: imneov <grantliu@yunify.com>
@imneov
Copy link
Contributor Author

imneov commented Mar 10, 2022

@imneov Almost done ... just a few small changes and linter fixes ...

@mukundansundar Thank you kindly! I fix it.

@mukundansundar
Copy link
Collaborator

mukundansundar commented Apr 8, 2022 •
edited
Loading

@imneov Almost done ... just a few small changes and linter fixes ...

@mukundansundar Thank you kindly! I fix it.

@imneov Let me know if you will be able to address some of the requested changes ... Thanks

@imneov
fix: lint
96f1224 
Signed-off-by: 柳丰 <grantliu@yunify.com>
@imneov imneov force-pushed the feat/dashboard-supports-random-port branch from f619819 to 96f1224 Compare April 13, 2022 01:36
@imneov
Copy link
Contributor Author

imneov commented Apr 13, 2022

@imneov Almost done ... just a few small changes and linter fixes ...

@mukundansundar Thank you kindly! I fix it.

@imneov Let me know if you will be able to address some of the requested changes ... Thanks

HI，@mukundansundar ~ I have responded to the modification in the commit below, what else should I do?

@pravinpushkar
Copy link
Contributor

LGTM
However, There will be a need to update the documentation here - https://docs.dapr.io/reference/cli/dapr-dashboard/ .
@imneov Would you be able to create a doc issue here - https://github.com/dapr/docs/issues and a small PR to update the same.

shubham1172
shubham1172 suggested changes Apr 18, 2022
View reviewed changes
Copy link
Member

@shubham1172 shubham1172 left a comment

Choose a reason for hiding this comment

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

Please address the comments.

@imneov imneov force-pushed the feat/dashboard-supports-random-port branch from 6adc0c7 to c7bd055 Compare May 2, 2022 14:00
shubham1172
shubham1172 previously approved these changes May 9, 2022
View reviewed changes
yaron2
yaron2 previously requested changes May 9, 2022
View reviewed changes
@imneov
feat: fix dot lint
2bbac25 
Signed-off-by: 柳丰 <grantliu@yunify.com>
@imneov imneov force-pushed the feat/dashboard-supports-random-port branch from 5f3f852 to 2bbac25 Compare May 12, 2022 01:07
@dapr-bot
Copy link
Collaborator

dapr-bot commented Jul 9, 2022

This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

@dapr-bot dapr-bot added the stale label Jul 9, 2022
@dapr-bot
Copy link
Collaborator

This pull request has been automatically closed because it has not had activity in the last 37 days. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

@dapr-bot dapr-bot closed this Jul 16, 2022
@pravinpushkar pravinpushkar reopened this Aug 1, 2022
@dapr-bot dapr-bot removed the stale label Aug 1, 2022
@pravinpushkar
Copy link
Contributor

@imneov Please update the branch and resolve the conflicts. Additionally, Please update the documentation also.

imneov and others added 4 commits August 30, 2022 17:31
@imneov
Copy link
Contributor Author

imneov commented Sep 10, 2022

@imneov Please update the branch and resolve the conflicts. Additionally, Please update the documentation also.

@pravinpushkar It's done.

@imneov
fix: Fix build error for int to string conversion
60d94c4 
Signed-off-by: liufeng <liuf@kubesphere.io>
@imneov imneov force-pushed the feat/dashboard-supports-random-port branch from 0e95af5 to 60d94c4 Compare September 26, 2022 01:56
@imneov
Copy link
Contributor Author

imneov commented Sep 27, 2022

@pravinpushkar HI~ Is there anything else I need to do?

@pravinpushkar
Copy link
Contributor

@pravinpushkar HI~ Is there anything else I need to do?

There are few PR checks failing, could you please have a look? For now I have re-triggered some of them. Hopefully E2E- Upgrade path tests should pass as it looked temporary failure. But other one should not have failed.

@imneov
Copy link
Contributor Author

imneov commented Sep 30, 2022

@pravinpushkar HI~ Is there anything else I need to do?

There are few PR checks failing, could you please have a look? For now I have re-triggered some of them. Hopefully E2E- Upgrade path tests should pass as it looked temporary failure. But other one should not have failed.

HI, @pravinpushkar ~ I don't find the failing PR checks,could you give me a link?

@mukundansundar mukundansundar dismissed yaron2’s stale review September 30, 2022 04:17

Review comments addressed

@mukundansundar mukundansundar merged commit 3bf4b5e into dapr:master Sep 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mukundansundar mukundansundar mukundansundar approved these changes

@yaron2 yaron2 yaron2 left review comments

@shubham1172 shubham1172 shubham1172 left review comments

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

Successfully merging this pull request may close these issues.

The port number of the dashboard supports automatic selection.
6 participants
@imneov @mukundansundar @pravinpushkar @dapr-bot @yaron2 @shubham1172