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

Only use LONGSIZE in rpm.info if available. Otherwise, use SIZE. #31445

Merged
merged 4 commits into from Feb 24, 2016

Conversation

Projects
None yet
3 participants
@rallytime
Copy link
Contributor

commented Feb 24, 2016

Fixes #31366

I have tested this manually on CentOS 6 and CentsOS 5, but I want to write an automated test. I started a test in #31439, which I want to add to here once that is merged. At that time I'll rebase and add the test.

Without this change, a call to lowpkg.info gets an error from the cmd.run_all call stating a tag is invalid, which is LONGSIZE. This change fixes error and returns successfully. Also added a more helpful error catch so we don't just return None if there is information in the stderr, but we still got a retcode: 0.

# salt-call --local lowpkg.info git
<snipped>
local:
    ----------
    git:
        ----------
        arch:
            x86_64
        build_date:
            2013-11-21T19:57:31Z
        build_date_time_t:
            1385081851
        build_host:
            lisse.hasselt.wieers.com
        description:
            GIT comes in two layers. The bottom layer is merely an extremely fast
            and flexible filesystem-based database designed to store directory trees
            with regard to their history. The top layer is a SCM-like tool which
            enables human beings to work with the database in a manner to a degree
            similar to other SCM tools (like CVS, BitKeeper or Monotone).
        group:
            Development/Tools
        install_date:
            2016-02-23T16:27:28Z
        install_date_time_t:
            1456262848
        license:
            GPL
        packager:
            Dag Wieers <dag@wieers.com>
        release:
            1.el5.rf
        relocations:
            (not relocatable)
        signature:
            DSA/SHA1, Fri Nov 22 01:25:29 2013, Key ID a20e52146b8d79e6
        size:
            17639877
        source_rpm:
            git-1.7.12.4-1.el5.rf.src.rpm
        summary:
            Git core and tools
        url:
            http://git-scm.com/
        vendor:
            Dag Apt Repository, http://dag.wieers.com/apt/
        version:
            1.7.12.4
@cachedout

This comment has been minimized.

Copy link
Collaborator

commented Feb 24, 2016

@terminalmage Could you review this, please?

# LONGSIZE is not a valid tag for all versions of rpm. If LONGSIZE isn't
# available, then we can just use SIZE for older versions. See Issue #31366.
rpm_tags = __salt__['cmd.run_all']('rpm --querytags')
rpm_tags = rpm_tags.get('stdout').split('\n')

This comment has been minimized.

Copy link
@terminalmage

terminalmage Feb 24, 2016

Contributor

Since we're not paying attention to the retcode or other information that is returned in cmd.run_all, the above two lines can be more concisely written as follows:

rpm_tags = __salt__['cmd.run_stdout'](
    ['rpm', '--querytags'],
    python_shell=False).splitlines()

@rallytime rallytime force-pushed the rallytime:fix-31366 branch from 9a9da33 to 9965fe1 Feb 24, 2016

@rallytime

This comment has been minimized.

Copy link
Contributor Author

commented Feb 24, 2016

@terminalmage Good call. I've updated that. I have also updated the test for this function to include RedHat and Suse distros.

@terminalmage

This comment has been minimized.

Copy link
Contributor

commented Feb 24, 2016

Looks great, pending tests passing this is good to go. Thanks!

cachedout pushed a commit that referenced this pull request Feb 24, 2016

Mike Place
Merge pull request #31445 from rallytime/fix-31366
Only use LONGSIZE in rpm.info if available. Otherwise, use SIZE.

@cachedout cachedout merged commit 987dd89 into saltstack:2015.8 Feb 24, 2016

5 checks passed

default Merged build finished.
Details
jenkins/salt-pr-clone Salt PR - Clone Repository #14052 — SUCCESS
Details
jenkins/salt-pr-lint-n Salt PR - Code Lint #13732 — SUCCESS
Details
jenkins/salt-pr-rs-cent7-n Salt PR - RS CentOS 7 #12792 — SUCCESS
Details
jenkins/salt-pr-rs-ubuntu14.04-n Salt PR - RS Ubuntu 14 #10114 — SUCCESS
Details

@rallytime rallytime deleted the rallytime:fix-31366 branch Feb 29, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.