Fix for installation issues requiring sudo and no-access to etc/bash_completion.d #128
Conversation
…ich enables autocomplete) - Removed the now unnecessary entry in etc/bash_completion.d/
|
@rozuur Can you please review this pull request? It fixes the installation issues by removing the need for an entry in /etc/bash_completion.d/ which multiple people have complained about. Removing that also allows sudo-free installation. Autocomplete is handled by setup.py entry point [1] PS The build is failed because of the encrypted env variables. |
|
Will review this week |
| except Exception as e: | ||
| fail('[Runtime Exception] ', exc_info=e, stacktrace=True) | ||
| def main(): | ||
| try: |
There was a problem hiding this comment.
why is this try block required here?
There was a problem hiding this comment.
Simply for cleaner exceptions and sticking with current behaviour. Currently only the [Error] message is printed out and process exits. But with this change the exception trace was also being printed. This simply swallows that (though verbose logging will show the exception stacktrace if needed)
| @@ -1 +0,0 @@ | |||
| include data/bash-completion/s4cmd | |||
There was a problem hiding this comment.
this is only used with sdist command used for distribution should this be removed? https://docs.python.org/2/distutils/sourcedist.html#the-manifest-in-template
There was a problem hiding this comment.
Since the file is deleted, it is no longer needed. When it doesn't exist the default manifest is used: https://docs.python.org/2/distutils/sourcedist.html#manifest which works for s4cmd since it's just py files which are included by default
|
Yes. The bash completion is only for the s4cmd command though similar to current behaviour. To test, installed on a fresh machine with |
s4cmd.py
Outdated
| progress('') # Clear progress message before exit. | ||
| clean_tempfiles() | ||
| progress('') # Clear progress message before exit. | ||
| except: |
There was a problem hiding this comment.
use except Exception: instead of naked except
|
Done. |
|
lgtm, after build success you can promote the code. |
…completion.d (bloomreach#128) * - Create main method and update setup.py to call main as a script (which enables autocomplete) - Removed the now unnecessary entry in etc/bash_completion.d/ * Delete s4cmd bash script and remove references to it * [Minor] Catch Exception instead of naked catch
Create main method and update setup.py to call main as a script (which enables autocomplete)
Removed the now unnecessary entry in etc/bash_completion.d/
Tested on a fresh VM and in virtualenv