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

Logs - replace forward slash (/) with underscore #268

Merged
merged 2 commits into from
Mar 7, 2020
Merged

Conversation

drehelis
Copy link
Contributor

@drehelis drehelis commented Mar 7, 2020

Hi,

I was playing with script-server for the first time today and noticed you cannot use '/' in the script name as it breaks the logging on the filesystem due to... well, obvious reasons :-)

FileNotFoundError: [Errno 2] No such file or directory: 'logs/processes/Group_add/remove_a_member_<host>_200307_134140.log'

There might be a better approach to do this as more chars may break this.

For your consideration

FileNotFoundError: [Errno 2] No such file or directory: 'logs/processes/Group_add/remove_a_member_<host>_200307_134140.log'
@bugy
Copy link
Owner

bugy commented Mar 7, 2020

Nice catch!
The fix looks good to me, so far after 3 years there were no complains about other chars :)

Thanks a lot!

@bugy
Copy link
Owner

bugy commented Mar 7, 2020

@autogun hmm, I had wrong tests, now they are failing because of wrongly expected name

@drehelis
Copy link
Contributor Author

drehelis commented Mar 7, 2020

@bugy IMHO, back & forward slashes should be avoided when it comes to naming conventions. See if agree to the test fix :)

@bugy
Copy link
Owner

bugy commented Mar 7, 2020

@autogun ye, it's better to avoid slashes. Don't know why I used it for the test :)
Thanks for the fix!

@bugy bugy merged commit dbd3f50 into bugy:master Mar 7, 2020
@bugy bugy added the bug label Mar 16, 2020
@bugy bugy added this to the 1.16.0 milestone Mar 16, 2020
@bugy bugy added the resolved label Mar 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants