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

partially unify public functions signature for pkg and lowpkg #51973

Merged
merged 2 commits into from
Apr 18, 2019

Conversation

aplanas
Copy link
Contributor

@aplanas aplanas commented Mar 5, 2019

What does this PR do?

The virtual module 'pkg' is heavily used by other states, like the
'pkg' state, so differences in the signature of public functions
can generate errors like:

TypeError: xxx() got an unexpected keyword argument 'yyy'

The fix is to add the parameter 'yyy' into all versions of the
function 'xxx()' in all the 'pkg' virtual modules, but this
approach is cumbersome and not very resilient, as any refactoring
on the public interface or in the pkg state module will break
again the already agreed contract.

This patch append **kwarg in most of the public methods on virtual
module pkg where it is missing, fixing the described issue and
making more easy the refactoring of the code.

Also do the same for 'lowpkg'

aplanas added a commit to aplanas/salt-1 that referenced this pull request Mar 6, 2019
* Add root and no_recommends parameters in the public interface for Zypper and RPM (lowpkg)
  saltstack/salt#50125

* Add root parameter to useradd, shadow and groupadd
  saltstack/salt#50175

* cmd: Add root parameter for wait and run states
  saltstack/salt#50302

* systemd: add optional root parameter
  saltstack/salt#50380

* Add new chroot module
  https://github.com/openSUSE/salt/pull/50418

* Add new module freezer
  saltstack/salt#50452

* btrfs: add all subvolume commands
  saltstack/salt#50541

* btrfs: add new btrfs state
  saltstack/salt#50635

* zypper: demote log from error to warning
  saltstack/salt#50671

* blkid: add search by token
  saltstack/salt#50706

* mount: add fstab_{present,absent} states
  saltstack/salt#50725

* btrfs: add option to not set subvolumes as default
  saltstack/salt#50801

* Add disk_set and disk_toggle functions, and update valid partition flags
  saltstack/salt#50834

* disk: support setting FAT size for format_
  saltstack/salt#51074

* parted: fix set_ valid flags comment.
  saltstack/salt#51704

* grains/core: ignore HOST_NOT_FOUND errno in fqdns()
  saltstack/salt#51706

* cmdmod: add 'binds' parameter in run_chroot
  saltstack/salt#51871

* mount: fix extra -t parameter
  saltstack/salt#51905

* lvm: be quiet when a pv, lv or vg is not expected
  saltstack/salt#51929

* linux_lvm: clean error in pvcreate and pvremove
  saltstack/salt#51954

* blockdev: hide blkid errors when are expected
  saltstack/salt#51956

* partially unify public functions signature for pkg and lowpkg
  saltstack/salt#51973

* extmods: add utils directories in sys.path
  saltstack/salt#52001
@aplanas aplanas force-pushed the fix_zypper branch 3 times, most recently from 426daf3 to 173c649 Compare March 18, 2019 09:07
The virtual module 'pkg' is heavily used by other states, like the
'pkg' state, so differences in the signature of public functions
can generate errors like:

  TypeError: xxx() got an unexpected keyword argument 'yyy'

The fix is to add the parameter 'yyy' into all versions of the
function 'xxx()' in all the 'pkg' virtual modules, but this
approach is cumbersome and not very resilient, as any refactoring
on the public interface or in the pkg state module will break
again the already agreed contract.

This patch append **kwarg in most of the public methods on virtual
module pkg where it is missing, fixing the described issue and
making more easy the refactoring of the code.
In the same way that the previous commit for pkg, add **kwargs
into the public function signature for lowpkg virtual module, to
unify the different signatures.
@dwoz dwoz merged commit 9864c59 into saltstack:develop Apr 18, 2019
@aplanas aplanas deleted the fix_zypper branch April 24, 2019 08:23
aplanas added a commit to aplanas/salt-1 that referenced this pull request Jun 5, 2019
* Add root and no_recommends parameters in the public interface for Zypper and RPM (lowpkg)
  saltstack/salt#50125

* Add root parameter to useradd, shadow and groupadd
  saltstack/salt#50175

* cmd: Add root parameter for wait and run states
  saltstack/salt#50302

* systemd: add optional root parameter
  saltstack/salt#50380

* Add new chroot module
  https://github.com/openSUSE/salt/pull/50418

* Add new module freezer
  saltstack/salt#50452

* btrfs: add all subvolume commands
  saltstack/salt#50541

* btrfs: add new btrfs state
  saltstack/salt#50635

* zypper: demote log from error to warning
  saltstack/salt#50671

* blkid: add search by token
  saltstack/salt#50706

* mount: add fstab_{present,absent} states
  saltstack/salt#50725

* btrfs: add option to not set subvolumes as default
  saltstack/salt#50801

* Add disk_set and disk_toggle functions, and update valid partition flags
  saltstack/salt#50834

* disk: support setting FAT size for format_
  saltstack/salt#51074

* parted: fix set_ valid flags comment.
  saltstack/salt#51704

* grains/core: ignore HOST_NOT_FOUND errno in fqdns()
  saltstack/salt#51706

* cmdmod: add 'binds' parameter in run_chroot
  saltstack/salt#51871

* mount: fix extra -t parameter
  saltstack/salt#51905

* lvm: be quiet when a pv, lv or vg is not expected
  saltstack/salt#51929

* linux_lvm: clean error in pvcreate and pvremove
  saltstack/salt#51954

* blockdev: hide blkid errors when are expected
  saltstack/salt#51956

* partially unify public functions signature for pkg and lowpkg
  saltstack/salt#51973

* extmods: add utils directories in sys.path
  saltstack/salt#52001
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