Skip to content

Commit

Permalink
Remove fact caching
Browse files Browse the repository at this point in the history
As of the previous commit, fact caching has very little benefit but
adds a significant amount of complexity to pyinfra. The diff alone
should highlight how great this is :)
  • Loading branch information
Fizzadar committed Jul 23, 2023
1 parent 07e7e34 commit a46443f
Show file tree
Hide file tree
Showing 21 changed files with 20 additions and 507 deletions.
75 changes: 10 additions & 65 deletions pyinfra/api/facts.py
Original file line number Diff line number Diff line change
Expand Up @@ -186,8 +186,6 @@ def get_fact(
kwargs: Optional[Any] = None,
ensure_hosts: Optional[Any] = None,
apply_failed_hosts: bool = True,
fact_hash: Optional[Any] = None,
use_cache: bool = True,
):
if issubclass(cls, ShortFactBase):
return get_short_facts(
Expand All @@ -198,24 +196,17 @@ def get_fact(
kwargs=kwargs,
ensure_hosts=ensure_hosts,
apply_failed_hosts=apply_failed_hosts,
fact_hash=fact_hash,
use_cache=use_cache,
)

with host.facts_lock:
if use_cache and fact_hash and fact_hash in host.facts:
return host.facts[fact_hash]

return _get_fact(
state,
host,
cls,
args,
kwargs,
ensure_hosts,
apply_failed_hosts,
fact_hash,
)
return _get_fact(
state,
host,
cls,
args,
kwargs,
ensure_hosts,
apply_failed_hosts,
)


def _get_fact(
Expand All @@ -226,7 +217,6 @@ def _get_fact(
kwargs: Optional[Dict] = None,
ensure_hosts: Optional[Any] = None,
apply_failed_hosts: bool = True,
fact_hash: Optional[Any] = None,
):
fact = cls()
name = fact.name
Expand Down Expand Up @@ -333,8 +323,6 @@ def _get_fact(
if not status and not ignore_errors and apply_failed_hosts:
state.fail_hosts({host})

if fact_hash:
host.facts[fact_hash] = data
return data


Expand All @@ -352,47 +340,4 @@ def get_host_fact(
args: Optional[List] = None,
kwargs: Optional[Dict] = None,
):
fact_hash = _get_fact_hash(state, host, cls, args, kwargs)
return get_fact(state, host, cls, args=args, kwargs=kwargs, fact_hash=fact_hash)


def reload_host_fact(
state: "State",
host: "Host",
cls,
args: Optional[List] = None,
kwargs: Optional[Dict] = None,
):
fact_hash = _get_fact_hash(state, host, cls, args, kwargs)
return get_fact(
state,
host,
cls,
args=args,
kwargs=kwargs,
fact_hash=fact_hash,
use_cache=False,
)


def create_host_fact(
state: "State",
host: "Host",
cls,
data,
args: Optional[List] = None,
kwargs: Optional[Dict] = None,
):
fact_hash = _get_fact_hash(state, host, cls, args, kwargs)
host.facts[fact_hash] = data


def delete_host_fact(
state: "State",
host: "Host",
cls,
args: Optional[List] = None,
kwargs: Optional[Dict] = None,
):
fact_hash = _get_fact_hash(state, host, cls, args, kwargs)
host.facts.pop(fact_hash, None)
return get_fact(state, host, cls, args=args, kwargs=kwargs)
27 changes: 1 addition & 26 deletions pyinfra/api/host.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,13 @@
from typing import TYPE_CHECKING, Any, Callable, Dict, Generator, List, Optional, Union

import click
from gevent.lock import BoundedSemaphore

from pyinfra import logger
from pyinfra.connectors.util import remove_any_sudo_askpass_file

from .connectors import get_execution_connector
from .exceptions import ConnectError
from .facts import create_host_fact, delete_host_fact, get_host_fact, reload_host_fact
from .facts import get_host_fact

if TYPE_CHECKING:
from pyinfra.api.inventory import Inventory
Expand Down Expand Up @@ -129,11 +128,6 @@ def __init__(
self.connector_data = {}
self.current_op_global_kwargs = {}

# Fact data store
# TODO: how to not have Any here?
self.facts: Dict[str, Any] = {}
self.facts_lock = BoundedSemaphore()

# Append only list of operation hashes as called on this host, used to
# generate a DAG to create the final operation order.
self.op_hash_order: List[str] = []
Expand Down Expand Up @@ -254,25 +248,6 @@ def get_fact(self, name_or_cls, *args, **kwargs):
"""
return get_host_fact(self.state, self, name_or_cls, args=args, kwargs=kwargs)

def reload_fact(self, name_or_cls, *args, **kwargs):
"""
Get a fact for this host without using any cached value, always re-fetch the fact data
from the host and then cache it.
"""
return reload_host_fact(self.state, self, name_or_cls, args=args, kwargs=kwargs)

def create_fact(self, name_or_cls, data=None, kwargs=None):
"""
Create a new fact for this host in the fact cache.
"""
return create_host_fact(self.state, self, name_or_cls, data, kwargs=kwargs)

def delete_fact(self, name_or_cls, kwargs=None):
"""
Remove an existing fact for this host in the fact cache.
"""
return delete_host_fact(self.state, self, name_or_cls, kwargs=kwargs)

# Connector proxy
#

Expand Down
23 changes: 1 addition & 22 deletions pyinfra/operations/apt.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
Manage apt packages and repositories.
"""

from datetime import datetime, timedelta
from datetime import timedelta
from urllib.parse import urlparse

from pyinfra import host, state
Expand Down Expand Up @@ -78,10 +78,6 @@ def key(src=None, keyserver=None, keyid=None):
yield "(wget -O - {0} || curl -sSLf {0}) | apt-key add -".format(src)
else:
yield "apt-key add {0}".format(src)

if keyid:
for kid in keyid:
existing_keys[kid] = {}
else:
host.noop("All keys from {0} are already available in the apt keychain".format(src))

Expand All @@ -98,8 +94,6 @@ def key(src=None, keyserver=None, keyid=None):
keyserver,
" ".join(needed_keys),
)
for kid in keyid:
existing_keys[kid] = {}
else:
host.noop(
"Keys {0} are already available in the apt keychain".format(
Expand Down Expand Up @@ -149,7 +143,6 @@ def repo(src, present=True, filename=None):
src,
escape_regex_characters=True,
)
apt_sources.append(repo)

# Exists and we don't want it
elif is_present and not present:
Expand All @@ -160,8 +153,6 @@ def repo(src, present=True, filename=None):
assume_present=True,
escape_regex_characters=True,
)
apt_sources.remove(repo)

else:
host.noop(
'apt repo "{0}" {1}'.format(
Expand Down Expand Up @@ -266,9 +257,6 @@ def deb(src, present=True, force=False):
# Now reinstall, and critically configure, the package - if there are still
# missing deps, now we error
yield "dpkg --force-confdef --force-confold -i {0}".format(src)

if info:
current_packages[info["name"]] = [info.get("version")]
else:
host.noop("deb {0} is installed".format(original_src))

Expand All @@ -279,7 +267,6 @@ def deb(src, present=True, force=False):
noninteractive_apt("remove", force=force),
info["name"],
)
current_packages.pop(info["name"])
else:
host.noop("deb {0} is not installed".format(original_src))

Expand Down Expand Up @@ -326,14 +313,6 @@ def update(cache_time=None):
# cache_time to work.
if cache_time:
yield "touch {0}".format(APT_UPDATE_FILENAME)
if cache_info is None:
host.create_fact(
File,
kwargs={"path": APT_UPDATE_FILENAME},
data={"mtime": datetime.utcnow()},
)
else:
cache_info["mtime"] = datetime.utcnow()


_update = update # noqa: E305
Expand Down
2 changes: 0 additions & 2 deletions pyinfra/operations/brew.py
Original file line number Diff line number Diff line change
Expand Up @@ -192,11 +192,9 @@ def tap(src, present=True):
host.noop("tap {0} already exists".format(src))
else:
yield "brew tap {0}".format(src)
taps.append(src)

elif not present:
if is_tapped:
yield "brew untap {0}".format(src)
taps.remove(src)
else:
host.noop("tap {0} does not exist".format(src))
Loading

0 comments on commit a46443f

Please sign in to comment.