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

FactCache updates #5534

Closed
wants to merge 37 commits into from
Closed

FactCache updates #5534

wants to merge 37 commits into from

Conversation

sivel
Copy link
Member

@sivel sivel commented Jan 7, 2014

  • The get method of a dict does not internally call __getitem__, so add a get method that does

Currently utils.update_hash uses FactCache.get() which does not internally utilize the __getitem__ method, so it is always returning {}

  • When trying to update subkeys of self.SETUP_CACHE make sure to use utils.update_hash

These are just a few missed from 3ba0ea0

sivel and others added 30 commits January 6, 2014 09:13
* Support state=absent with group or a group meta key
* Add auto_increment attribute to allow specification of a group without forcing an auto incrementing host number
* Add additional example that showcases exact_count functionality
* Don't use a file handle for files to upload to the server, read in the contents to avoid an issue where the file handle wasn't reset and an empty file was being uploaded
Signed-off-by: Brian Coca <briancoca+dev@gmail.com>
Signed-off-by: Brian Coca <briancoca+dev@gmail.com>
fixing
Signed-off-by: Brian Coca <briancoca+dev@gmail.com>
key
Signed-off-by: Brian Coca <briancoca+dev@gmail.com>
Signed-off-by: Brian Coca <briancoca+dev@gmail.com>
collections.mutablemappings as alternative
Signed-off-by: Brian Coca <briancoca+dev@gmail.com>
Signed-off-by: Brian Coca <briancoca+dev@gmail.com>
fixed issue with setter
Signed-off-by: Brian Coca <briancoca+dev@gmail.com>
new facts or default (dict).

corrected setdefault param mismatch

Signed-off-by: Brian Coca <briancoca+dev@gmail.com>
Signed-off-by: Brian Coca <briancoca+dev@gmail.com>
behaviour)

added __contains__ method in cache

currently merges local cache with facts, for some reason 'in' doesn't
seem to match existing keys.

Signed-off-by: Brian Coca <briancoca+dev@gmail.com>
- now checks if cachd_dir is actualy a dir
- cached function returns list of cached keys/hosts
- now stats file to se if it has 'expired'

Signed-off-by: Brian Coca <briancoca+dev@gmail.com>
Signed-off-by: Brian Coca <briancoca+dev@gmail.com>
Signed-off-by: Brian Coca <briancoca+dev@gmail.com>
Signed-off-by: Brian Coca <briancoca+dev@gmail.com>
…arning

Fixes ansible#5513. set is built-in since 2.4 and deprecated since 2.6
…po. symetric should be symmetric. I fixed the typo and verified that the filter in core.py is spelled correctly.
While looking into filters, I noticed that the documentation has a typo....
…ng the need for 'bypass'.

Defining the 'keys' method of the plugin is also likely adviseable next.
…mplementation is as efficient as we'd want it to be.
Adding support for detecting RHEV Hypervisor in ansible_virtualization_type
jctanner and others added 7 commits January 7, 2014 07:10
* The get method of a dict does not internally call __getitem__, so add a get method that does
* When trying to update subkeys of self.SETUP_CACHE make sure to use utils.update_hash
@sivel
Copy link
Member Author

sivel commented Jan 7, 2014

Actually, this doesn't seem to be the full fix for the 'memory' FactCache plugin. It works for a memcached plugin I am working on, but not for memory. Still looking, to see if I can figure out the issue there too.

@mpdehaan
Copy link
Contributor

mpdehaan commented Jan 7, 2014

Looks like we don't want this as this no longer defers the implementation of getitem to the plugin.

I'm looking at the email on the thread now.

@mpdehaan
Copy link
Contributor

mpdehaan commented Jan 7, 2014

Can you share an example of where this causes a problem with a playbook?

@sivel
Copy link
Member Author

sivel commented Jan 7, 2014

The following playbook suffers from the issue

---
- hosts: localhost
  gather_facts: yes
  connection: local
  tasks:
  - set_fact:
      foo: bar

  - command: echo test
    register: echo

  - debug: var=foo

  - debug: var=echo

In both cases, foo and echo are not interpreted:

PLAY [localhost] **************************************************************

GATHERING FACTS ***************************************************************
ok: [127.0.0.1]

TASK: [set_fact ] *************************************************************
ok: [127.0.0.1]

TASK: [command echo test] *****************************************************
changed: [127.0.0.1]

TASK: [debug var=foo] *********************************************************
ok: [127.0.0.1] => {
    "foo": "{{ foo }}"
}

TASK: [debug var=echo] ********************************************************
ok: [127.0.0.1] => {
    "echo": "{{ echo }}"
}

PLAY RECAP ********************************************************************
127.0.0.1                  : ok=5    changed=1    unreachable=0    failed=0

This is straight off of devel:

ansible-playbook 1.5 (devel 3ba0ea064d) last updated 2014/01/07 12:21:30 (GMT -500)

This happens with the default 'memory' fact cache plugin.

I also experience this with my memcached plugin, which you can see at https://github.com/sivel/ansible/compare/memcached-factcache

The fix in this pull request resolves the issue with my memcached plugin, but does not fix the issue with the memory plugin. I haven't figured out why yet, but I can see that with the following debug, these vars are indeed in the value from utils.update_hash:

diff --git a/lib/ansible/utils/__init__.py b/lib/ansible/utils/__init__.py
index d32506e..e4114c2 100644
--- a/lib/ansible/utils/__init__.py
+++ b/lib/ansible/utils/__init__.py
@@ -963,6 +963,7 @@ def update_hash(hash, key, new_value):

     value = hash.get(key, {})
     value.update(new_value)
+    print value
     hash[key] = value

@mpdehaan
Copy link
Contributor

mpdehaan commented Jan 7, 2014

So top part is good, but I think setitem is still called for the bottom part.

@mpdehaan
Copy link
Contributor

mpdehaan commented Jan 7, 2014

We have reset to before this commit so shouldn't be a probelm.

We want to do some refactoring before I think we're ready for this.

@mpdehaan mpdehaan closed this Jan 7, 2014
@ansible ansible locked and limited conversation to collaborators Apr 24, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants