Skip to content

Conversation

@dasilvaca
Copy link

Description of the issue/feature this PR addresses:

Current behavior before PR:

Desired behavior after PR is merged:

Copy link
Member

Choose a reason for hiding this comment

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

This file should not be there

def install_many(self, names):
"""Install many packages."""
return utils.exec_cmd("yum install -y --quiet {}".format(" ".join(names)))
return utils.exec_cmd("yum install -y {}".format(" ".join(names))) # Before it was quiet
Copy link
Member

Choose a reason for hiding this comment

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

Why do you remove the quiet option?

Copy link
Author

Choose a reason for hiding this comment

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

To see if somehow one of the packages may fail. Also, during this step installation took so long, so it didn't actually seems to be doing anything, or, better said, it didn't give the sensation of progress.

"""Initialize backend."""
super(RPMPackage, self).__init__(dist_name)
if "centos" in dist_name:
if dist_name in ["centos", "oracle linux server"]:
Copy link
Member

Choose a reason for hiding this comment

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

Be careful here, you're changing the check logic for centos. This was not a strict comparison

Copy link
Author

Choose a reason for hiding this comment

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

Maybe a better comparison could be to use the availability of the package manager. Anyways, it still didn't affect CentOS functionality at all. Do you suggest something different?

if distname in ["debian", "debian gnu/linux", "ubuntu", "linuxmint"]:
backend = DEBPackage
elif "centos" in distname:
elif distname in ["centos", "oracle linux server"]:
Copy link
Member

Choose a reason for hiding this comment

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

Be careful here, you're changing the check logic for centos. This was not a strict comparison

if ssl_protocol_parameter == "ssl_min_protocol":
ssl_protocols = "TLSv1"
if "centos" in utils.dist_name():
if utils.dist_name() in ["centos", "oracle linux server"]:
Copy link
Member

Choose a reason for hiding this comment

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

Be careful here, you're changing the check logic for centos. This was not a strict comparison

"""Additionnal variables."""
context = super(Uwsgi, self).get_template_context()
if package.backend.FORMAT == "deb":
if package.backend.FORMAT == "deb" or True:
Copy link
Member

Choose a reason for hiding this comment

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

Why are you doing this ?

Copy link
Author

Choose a reason for hiding this comment

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

On oracle linux the default usage of python was also python3. Then, I suggest this to be the default scenario, as I don't know in which distros "python36" is being used. (Reversing the logic might be a better solution: in those distro's special cases, use python36 instead of python3)

for appname in PRIMARY_APPS:
scripts.install(appname, config, args.upgrade, args.restore)
system.restart_service("cron")
system.restart_service("crond")
Copy link
Member

Choose a reason for hiding this comment

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

Will this work in all cases?

Copy link
Author

Choose a reason for hiding this comment

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

I could give it a try using Rocky, as CentOS is now unavailable. I haven't installed it on debian based distros.

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.

2 participants