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(user access): fix user access in some tasks (#17) #18

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

niyushabaghayi
Copy link
Collaborator

@niyushabaghayi niyushabaghayi commented Nov 26, 2023

Due to the problems with other ansible users than root, I have fixed them in the tasks which faced problems.

Please check them dear guys <3
PS, Again here, another PR, another review! I've missed it :`)

@niyushabaghayi niyushabaghayi linked an issue Nov 26, 2023 that may be closed by this pull request
@niyushabaghayi niyushabaghayi changed the title 🐛 fix(user access): fix user access in some tasks 🐛 fix(user access): fix user access in some tasks (#17) Nov 26, 2023
Copy link
Collaborator

@mostafaghadimi mostafaghadimi left a comment

Choose a reason for hiding this comment

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

Well done, Niyusha!

Thanks for your great contribution. I need your guidance in the code and already asked some questions. I would be thankful if it is possible for you to answer them.

changed_when: false
ignore_errors: yes # Use ignore_errors instead of failed_when
Copy link
Collaborator

Choose a reason for hiding this comment

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

Niyusha jan, would you please what is the difference has been made by this change? This is not that obvious for me.

Copy link
Collaborator

Choose a reason for hiding this comment

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

If our task cannot retrieve the "alacritty_latest_deb_url" we can find it out with a more clear message than just failing a task named "Get URL pointing to the latest alacritty .deb".
By the way I think it is unnecessary to split this task into two.

- name: Fail the task if wget fails
fail:
msg: "Failed to retrieve the latest alacritty .deb URL"
when: alacritty_latest_deb_url.rc != 0
Copy link
Collaborator

Choose a reason for hiding this comment

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

And why do we check this result and ignore_error in the previous step?

@@ -12,16 +12,16 @@

- name: Fetch .zshrc from remote machine
fetch:
src: /root/.zshrc
src: "{{ '/home/' + user + '/.zshrc' if user != 'root' else '/root/.zshrc' }}"
Copy link
Collaborator

Choose a reason for hiding this comment

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

can we handle it using ansible_user instead of set condition for it? How does the user is set? does it come from the ansible_user?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, variable {{ user }} comes from ansible_user which resides in /roles/shell/vars/main.yml.
Can we just use anisble_user for less complexity?

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.

Fix user access in some tasks
3 participants