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

Fix for Quote in username errors dapr init #972 #1322

Merged
merged 9 commits into from
Jun 26, 2023

Conversation

mohitpalsingh
Copy link
Contributor

@mohitpalsingh mohitpalsingh commented Jun 22, 2023

Description

dapr init fails to run with certain username on Windows.
This issue was caused because of an extra apostrophe or single quote in the username since powershell considers it as special character and hence creates syntax issues. To solve this, I created a function in '/utils/utils.go' to take any command and converts into powershell compatible command by appending another quote character after every such character. This makes the command powershell compatible. Now it doesn't throw any syntax error and runs as expected.

Closes #972

Testing

PS C:\Users\Carlin'tVeld> cd D:
PS D:> .\dapr.exe init
Making the jump to hyperspace...
Container images will be pulled from Docker Hub
Installing runtime version 1.11.1
Downloading binaries and setting up components...
Downloaded binaries and completed components set up.
daprd binary has been installed to C:\Users\Carlin'tVeld.dapr\bin.
dapr_placement container is running.
dapr_redis container is running.
dapr_zipkin container is running.
Use docker ps to check running containers.
Success! Dapr is up and running. To get started, go here: https://aka.ms/dapr-getting-started
PS D:>

@mohitpalsingh mohitpalsingh requested review from a team as code owners June 22, 2023 17:45
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.

@mohitpalsingh thanks for the contribution.
Can you add unit tests for this?

@mohitpalsingh
Copy link
Contributor Author

@mohitpalsingh thanks for the contribution. Can you add unit tests for this?

yes sure. Will do that tomorrow.

@codecov
Copy link

codecov bot commented Jun 23, 2023

Codecov Report

Merging #1322 (0c45ce1) into master (a4f924f) will decrease coverage by 0.01%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master    #1322      +/-   ##
==========================================
- Coverage   26.82%   26.81%   -0.01%     
==========================================
  Files          39       39              
  Lines        3881     3882       +1     
==========================================
  Hits         1041     1041              
- Misses       2766     2767       +1     
  Partials       74       74              
Impacted Files Coverage Δ
pkg/standalone/standalone.go 4.46% <0.00%> (-0.01%) ⬇️

utils/utils.go Outdated Show resolved Hide resolved
Signed-off-by: Mohit Pal Singh <[email protected]>
utils/utils.go Outdated Show resolved Hide resolved
utils/utils.go Outdated Show resolved Hide resolved
mohitpalsingh and others added 2 commits June 23, 2023 21:52
Co-authored-by: Shubham Sharma <[email protected]>
Signed-off-by: Mohit Pal Singh <[email protected]>
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.

Thanks @mohitpalsingh for your first contribution, can you just fix the lint failure, overall PR LGTM.

Signed-off-by: Mohit Pal Singh <[email protected]>
@mohitpalsingh
Copy link
Contributor Author

Thanks @mohitpalsingh for your first contribution, can you just fix the lint failure, overall PR LGTM.

Thank you for the feedback and acknowledging my contribution. I appreciate your support. I have addressed the lint failure and made the necessary fixes. Your approval of the overall pull request means a lot to me. Thank you once again for your time and guidance.

utils/utils.go Outdated Show resolved Hide resolved
Signed-off-by: Shubham Sharma <[email protected]>
@yaron2 yaron2 merged commit 2751f5f into dapr:master Jun 26, 2023
@mukundansundar mukundansundar added this to the v1.12 milestone Sep 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Quote in username errors dapr init
4 participants