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

new filter human_bytes: convert a string (ex: 1Mo, 1K) into bytes #12074

Merged
merged 2 commits into from
Aug 24, 2016

Conversation

Yannig
Copy link
Contributor

@Yannig Yannig commented Aug 24, 2015

This PR replace a part of this one: #11896

Add human_bytes filter. This filter take an input string and convert it into bytes. Here's some examples:

  • 0.5 K:
ansible -m debug -a "msg={{'0.5K'|human_bytes}}" localhost
localhost | SUCCESS => {
    "changed": false, 
    "msg": "512"
}
  • 2M:
ansible -m debug -a "msg={{'2M'|human_bytes}}" localhost
localhost | SUCCESS => {
    "changed": false, 
    "msg": "2097152"
}
  • 1.5 G :
ansible -m debug -a "msg={{'1.5G'|human_bytes}}" localhost
localhost | SUCCESS => {
    "changed": false, 
    "msg": "1610612736"
}
  • A bad size unit (1 foo):
ansible -m debug -a "msg={{'1 foo'|human_bytes}}" localhost
localhost | FAILED! => {
    "failed": true, 
    "msg": "ERROR! human_bytes() can't interpret following string: 1 foo"
}
  • A bad number (.M):
ansible -m debug -a "msg={{'.M'|human_bytes}}" localhost
localhost | FAILED! => {
    "failed": true, 
    "msg": "ERROR! human_bytes() can't interpret following number: . (original input string: .M)"
}
  • Or no number at all:
ansible -m debug -a "msg={{'GG'|human_bytes}}" localhost
localhost | FAILED! => {
    "failed": true, 
    "msg": "ERROR! human_bytes() can't interpret following number:  (original input string: GG)"
}

As a side note, add tests for human_readable and human_bytes filters in test_filters.yml playbook.

@amenonsen
Copy link
Contributor

What is the use case for this filter?

@Yannig
Copy link
Contributor Author

Yannig commented Mar 26, 2016

@amenonsen I wrote several examples in test/integration/roles/test_filters/tasks/main.yml
Here is some example:

- name: Verify human_bytes
  tags: "human_bytes"
  assert:
    that:
        - "{{'0'|human_bytes}}        == 0"
        - "{{'0.1'|human_bytes}}      == 0"
        - "{{'0.9'|human_bytes}}      == 1"
        - "{{'1'|human_bytes}}        == 1"
        - "{{'10.00 KB'|human_bytes}} == 10240"
        - "{{   '11 MB'|human_bytes}} == 11534336"
        - "{{  '1.1 GB'|human_bytes}} == 1181116006"

I'm using this with getmountfrompath (#12066) to check if I have enough space on a given path before trying to do something on it. This way, I can avoid filesystem full.

Here's an example of possible combination:

- name: "Check /tmp space"
  hosts: localhost
  tasks:
    - debug: msg={{'/tmp'|getmountfrompath(ansible_mounts)}}
    - name: "Check there's enough space"
      fail: msg="Not enough space available"
      when: ('/tmp'|getmountfrompath(ansible_mounts)).size_available < '1M' | human_bytes

If you wan't, I can rework my PR to resolv conflicts against last version of Ansible.

@amenonsen
Copy link
Contributor

I did see the examples, but didn't find 1|human_bytes to be especially compelling. But the way you use it with getmountfrompath makes sense. I don't like the name, though. "human_bytes" sounds like it produces something for a human, not the other way around. (What I'd really like to see is something that can compare 1M to .size_available directly, but I don't know if that's possible to write such a comparator in jinja2 at all.) I'm not sure what name would be better than human_bytes. Does anyone have any ideas?

@Yannig
Copy link
Contributor Author

Yannig commented Mar 29, 2016

Is there's any way to make a survey and get some feedback?

@mavit
Copy link
Contributor

mavit commented Jul 18, 2016

Is there's any way to make a survey and get some feedback?

I guess you could do worse than to ask on the mailing list, https://groups.google.com/forum/#!forum/ansible-project

I'm assuming that it's only a matter of time before someone comes along with a requirement for k representing 10³ rather than 2¹⁰, so we should probably pick a name that accommodates that. How about unprefix_binary or binary_prefix_to_int? The equivalent hypothetical base-10 filter would be, say, unprefix_decimal or si_prefix_to_int.

@bcoca bcoca added the needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html label Aug 23, 2016
return pretty_bytes(size, isbits, unit)

def string_to_bytes(self, size, default_unit = None):
return string_to_bytes(size, default_unit)
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't have a problem with _check_type_bytes simply calling the toplevel string_to_bytes(); no need for this method. (Wish we could do the same with pretty_bytes() but backwards compat... )

@abadger
Copy link
Contributor

abadger commented Aug 23, 2016

@Yannig we recently merged something similar from @bcoca. But when bcoca saw this he commented that he liked your code better. If you'd like to update this PR against what we have in the repo now we can look at merging it. (If no one responds after updating, feel free to ping on IRC -- I'm abadger1999)

@Yannig
Copy link
Contributor Author

Yannig commented Aug 24, 2016

@abadger I'll take a look.

New filter human_to_bytes.
@Yannig
Copy link
Contributor Author

Yannig commented Aug 24, 2016

@abadger ready for review

@bcoca bcoca removed the needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html label Aug 24, 2016
@bcoca
Copy link
Member

bcoca commented Aug 24, 2016

https://app.shippable.com/runs/57bd83031419e20f001b3adc/30/tests <= seems like it breaks the usage in facts.py

@bcoca bcoca added the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label Aug 24, 2016
base = 'bits'
suffix = ''

for suffix, limit in sorted(SIZE_RANGES.iteritems(), key=lambda item: -item[1]):
Copy link
Contributor

Choose a reason for hiding this comment

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

need to change this for python3 compatibility. We've imported the iteritems function from six earlier so we can just do this::

    for suffix, limit in sorted(iteritems(SIZE_RANGES), key=lambda item: -item[1]):

@abadger
Copy link
Contributor

abadger commented Aug 24, 2016

It's breakage when run on python3. I added a line note on how to fix it.

@Yannig
Copy link
Contributor Author

Yannig commented Aug 24, 2016

@abadger: I was not aware of this change in python 3. Fixed !

@bcoca bcoca removed the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label Aug 24, 2016
@bcoca
Copy link
Member

bcoca commented Aug 24, 2016

lgtm, +1

@abadger abadger merged commit 27b0f32 into ansible:devel Aug 24, 2016
@abadger
Copy link
Contributor

abadger commented Aug 24, 2016

merged! Thanks @Yannig for the PR and everyone that's taken the time to review!

@Yannig Yannig deleted the devel_human_bytes branch August 24, 2016 20:27
sereinity pushed a commit to sereinity-forks/ansible that referenced this pull request Jan 25, 2017
…sible#12074)

* Rework human_readable and human_to_bytes.
New filter human_to_bytes.

* Fix for python 3.
@ghost
Copy link

ghost commented Jul 16, 2017

Thanks for this great filter.

I just used it to set quotas for Ceph pools.

- name: Create quota for Ceph pools
  command: "/bin/ceph osd pool set-quota {{ item.name }} max_bytes {{ item.size |human_to_bytes }}"
  with_items: "{{ ceph.pools }}"
  become: yes
  tags:
    - pools   

@ansibot ansibot added feature This issue/PR relates to a feature request. and removed feature_pull_request labels Mar 4, 2018
@ansible ansible locked and limited conversation to collaborators Apr 26, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature This issue/PR relates to a feature request.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants