starred/proxmoxer

Fork 0

mirror of https://github.com/proxmoxer/proxmoxer.git synced 2026-04-25 07:06:00 +03:00

[GH-ISSUE #119] Implicitly remove None values from params #64

New issue

Closed

opened 2026-02-27 15:46:14 +03:00 by kerem · 4 comments

kerem commented

2026-02-27 15:46:14 +03:00

Owner

Originally created by @kristianheljas on GitHub (Nov 16, 2022).
Original GitHub issue: https://github.com/proxmoxer/proxmoxer/issues/119

When calling proxmoxer API methods with None values, it will behave differently when using https backend vs others.

The difference comes from requests library, which will implicitly remove None values from parameters.
On other backends the None values are implicitly converted to string, and providing them to the pvesh command.

A stupid, but simple example of this would be:

proxmox.storage.get(type=None) # Will fail, unless https backend is used

Would you guys accept a PR removing None values just as the requests module does?

This issue surfaced from a ansible.community issue about implementing local backend.

Originally created by @kristianheljas on GitHub (Nov 16, 2022). Original GitHub issue: https://github.com/proxmoxer/proxmoxer/issues/119 When calling proxmoxer API methods with `None` values, it will behave differently when using https backend vs others. The difference comes from requests library, which will [implicitly remove `None` values from parameters](https://github.com/psf/requests/blob/1e62a3ec18e19f85ddae03b4cbbdf0b4c62834c0/requests/sessions.py#L82-L86). On other backends the `None` values are implicitly converted to string, and providing them to the pvesh command. A stupid, but simple example of this would be: ```python proxmox.storage.get(type=None) # Will fail, unless https backend is used ``` Would you guys accept a PR removing `None` values just as the requests module does? This issue surfaced from a [ansible.community issue](https://github.com/ansible-collections/community.proxmox/issues/38) about implementing local backend.

kerem

2026-02-27 15:46:14 +03:00

closed this issue
added the
type:bug 🐞

backend:openssh

backend:ssh_paramiko

backend:local
labels

kerem commented

2026-02-27 15:46:15 +03:00

Author

Owner

@jhollowe commented on GitHub (Nov 18, 2022):

Sure, always willing to look at PRs! And standardizing usability between the different backends is definitely a goal.

@jhollowe commented on GitHub (Nov 18, 2022): Sure, always willing to look at PRs! And standardizing usability between the different backends is definitely a goal.

kerem commented

2026-02-27 15:46:15 +03:00

Author

Owner

@kristianheljas commented on GitHub (Nov 18, 2022):

Awsome!

Should same treatment applied to data as well?

I also discovered that tests utilizing mock_resource fixture do not actually check for data integrity. It's because MockSession.request() will always return statically defined {"data": {"key": "value"}}.
Should I adjust the behaviour of MockSession.request() as well?

@kristianheljas commented on GitHub (Nov 18, 2022): Awsome! Should same treatment applied to data as well? I also discovered that [tests utilizing `mock_resource` ](https://github.com/kristianheljas/proxmoxer/blob/63dccd40e2320e90b1fe11faa147c0d99c1af8a8/tests/test_core.py#L118-L197) fixture do not actually check for data integrity. It's because `MockSession.request()` will always return [statically defined `{"data": {"key": "value"}}`](https://github.com/proxmoxer/proxmoxer/blob/63dccd40e2320e90b1fe11faa147c0d99c1af8a8/tests/test_core.py#L310). Should I adjust the behaviour of `MockSession.request()` as well?

kerem commented

2026-02-27 15:46:15 +03:00

Author

Owner

@jhollowe commented on GitHub (Nov 19, 2022):

Yeah, please improve as much as you want. The tests are just basic right now and could use improvement.

@jhollowe commented on GitHub (Nov 19, 2022): Yeah, please improve as much as you want. The tests are just basic right now and could use improvement.

kerem commented

2026-02-27 15:46:15 +03:00

Author

Owner

@kristianheljas commented on GitHub (Nov 19, 2022):

@jhollowe Awesome, PR is here: https://github.com/proxmoxer/proxmoxer/pull/120

Thanks!

@kristianheljas commented on GitHub (Nov 19, 2022): @jhollowe Awesome, PR is here: https://github.com/proxmoxer/proxmoxer/pull/120 Thanks!

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

starred/proxmoxer#64

No description provided.

Rows
Columns