Add perl-modules as install dependency for cloudstack-agent #1495
Conversation
|
Quite horrible that we need those modules for a simple perl script of 58 lines (including license header and empty lines). Not related to your PR though, but a good candidate to be rewritten in bash or python |
|
@sverrirab how much effort would it be to port to Python? That seems far preferable for two reasons. First, as @eriweb rightly points out, it is a large dependency to take on for such a small script. Second, Python and bash are our scripting languages of choice. It does not seem sustainable to add runtimes/libraries for every scripting language people might want to use. Such sprawl will not only lead to bloat, but a system that fewer and fewer people can maintain. |
Required to run perl scripts that configure networking for VMs. That script fails silently if this is not installed.
|
I agree that this is not a good approach but it is the current one and it is broken. How about we commit this fix and I submit a new PR to master with a new python script? |
|
If this is broken right now and this fixes it, LGTM. But we should really ditch Perl and rewrite in Python or Bash. Can you afterwards submit a PR against master? |
|
do you want this change as a new PR against master or just the rewritten script in python on master? |
|
As this is a bug fix, I think it would be forward-merged into master. The next PR should be to rewrite the script in Python instead of Perl, and that would be on master only. |
|
tag:easypr |
|
Created new PR on master: #1526 |
|
LGTM |
1 similar comment
|
LGTM |
|
I think we should merge this one at this point. @sverrirab my understanding is that once #1533 is merged, this will be required. If that is the case, when this one is merged, maybe you can work into that PR the reverting of this PR. Does that make sense? |
…gent Add perl-modules as install dependency for cloudstack-agentRequired to run perl scripts that configure networking for VMs. * pr/1495: Add perl-modules as install dependency for cloudstack-agent Signed-off-by: Will Stevens <williamstevens@gmail.com>
…-python Convert patchviasocket to python (removes perl dependency for KVM agent)As requested here: #1495 No scripts are using perl so that install requirement can be removed. The new scripts are using standard python packages only. Includes extensive unit test. Note: perl-modules requirement is missing (fixed in mentioned PR) so do not merge that onto master. * pr/1533: Revert "Add perl-modules as install dependency for cloudstack-agent" patchviasocket improve error handling Convert patchviasocket to python (removes perl dependency for KVM agent) Signed-off-by: Will Stevens <williamstevens@gmail.com>
As requested here: apache#1495 No scripts are using perl so that install requirement can be removed. The new scripts are using standard python packages only. Includes extensive unit test.
Required to run perl scripts that configure networking for VMs.