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

Add the black Python code formatter #4313

Merged
merged 4 commits into from
Jun 30, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions build-aux/lint.mk
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ format/go: $(tools/golangci-lint)
# Python

lint-deps += $(OSS_HOME)/venv

lint-goals += lint/mypy
lint/mypy: $(OSS_HOME)/venv
set -e; { \
Expand All @@ -31,6 +32,16 @@ lint/mypy: $(OSS_HOME)/venv
.PHONY: lint/mypy
clean: .dmypy.json.rm .mypy_cache.rm-r

lint-goals += lint/black
lint/black: $(OSS_HOME)/venv
. $(OSS_HOME)/venv/bin/activate && black --check ./python/
.PHONY: lint/black

format-goals += format/black
format/black: $(OSS_HOME)/venv
. $(OSS_HOME)/venv/bin/activate && black ./python/
.PHONY: format/black

#
# Helm

Expand Down
2 changes: 2 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
[tool.black]
line-length = 100
2 changes: 1 addition & 1 deletion python/ambassador/VERSION.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
Commit = "MISSING(FILE)"
try:
with open(os.path.join(os.path.dirname(__file__), "..", "ambassador.version")) as version:
info = version.read().split('\n')
info = version.read().split("\n")
while len(info) < 2:
info.append("MISSING(VAL)")

Expand Down
158 changes: 93 additions & 65 deletions python/ambassador/ambscout.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,37 +26,38 @@ def __init__(self, logger, app: str, version: str, install_id: str) -> None:

self.events: List[Dict[str, Any]] = []

self.logger.info(f'LocalScout: initialized for {app} {version}: ID {install_id}')
self.logger.info(f"LocalScout: initialized for {app} {version}: ID {install_id}")

def report(self, **kwargs) -> dict:
self.events.append(kwargs)

mode = kwargs['mode']
action = kwargs['action']
mode = kwargs["mode"]
action = kwargs["action"]

now = datetime.datetime.now().timestamp()

kwargs['local_scout_timestamp'] = now
kwargs["local_scout_timestamp"] = now

if 'timestamp' not in kwargs:
kwargs['timestamp'] = now
if "timestamp" not in kwargs:
kwargs["timestamp"] = now

self.logger.info(f"LocalScout: mode {mode}, action {action} ({kwargs})")

return {
"latest_version": self.version,
"application": self.app,
"cached": False,
"notices": [ { "level": "WARNING", "message": "Using LocalScout, result is faked!" } ],
"timestamp": now
"notices": [{"level": "WARNING", "message": "Using LocalScout, result is faked!"}],
"timestamp": now,
}

def reset_events(self) -> None:
self.events = []


class AmbScout:
reTaggedBranch: ClassVar = re.compile(r'^v?(\d+\.\d+\.\d+)(-[a-zA-Z][a-zA-Z]\.\d+)?$')
reGitDescription: ClassVar = re.compile(r'-(\d+)-g([0-9a-f]+)$')
reTaggedBranch: ClassVar = re.compile(r"^v?(\d+\.\d+\.\d+)(-[a-zA-Z][a-zA-Z]\.\d+)?$")
reGitDescription: ClassVar = re.compile(r"-(\d+)-g([0-9a-f]+)$")

install_id: str
runtime: str
Expand All @@ -76,19 +77,25 @@ class AmbScout:
_latest_version: Optional[str] = None
_latest_semver: Optional[semantic_version.Version] = None

def __init__(self, install_id=None, update_frequency=datetime.timedelta(hours=12), local_only=False) -> None:
def __init__(
self, install_id=None, update_frequency=datetime.timedelta(hours=12), local_only=False
) -> None:
if not install_id:
install_id = os.environ.get('AMBASSADOR_CLUSTER_ID',
os.environ.get('AMBASSADOR_SCOUT_ID', "00000000-0000-0000-0000-000000000000"))
install_id = os.environ.get(
"AMBASSADOR_CLUSTER_ID",
os.environ.get("AMBASSADOR_SCOUT_ID", "00000000-0000-0000-0000-000000000000"),
)

self.install_id = install_id
self.runtime = "kubernetes" if os.environ.get('KUBERNETES_SERVICE_HOST', None) else "docker"
self.namespace = os.environ.get('AMBASSADOR_NAMESPACE', 'default')
self.runtime = "kubernetes" if os.environ.get("KUBERNETES_SERVICE_HOST", None) else "docker"
self.namespace = os.environ.get("AMBASSADOR_NAMESPACE", "default")

# Allow an environment variable to state whether we're in Edge Stack. But keep the
# existing condition as sufficient, so that there is less of a chance of breaking
# things running in a container with this file present.
self.is_edge_stack = parse_bool(os.environ.get('EDGE_STACK', 'false')) or os.path.exists('/ambassador/.edge_stack')
self.is_edge_stack = parse_bool(os.environ.get("EDGE_STACK", "false")) or os.path.exists(
"/ambassador/.edge_stack"
)
self.app = "aes" if self.is_edge_stack else "ambassador"
self.version = Version
self.semver = self.get_semver(self.version)
Expand All @@ -97,7 +104,9 @@ def __init__(self, install_id=None, update_frequency=datetime.timedelta(hours=12
# self.logger.setLevel(logging.DEBUG)

self.logger.debug("Ambassador version %s built from %s" % (Version, Commit))
self.logger.debug("Scout version %s%s" % (self.version, " - BAD SEMVER" if not self.semver else ""))
self.logger.debug(
"Scout version %s%s" % (self.version, " - BAD SEMVER" if not self.semver else "")
)
self.logger.debug("Runtime %s" % self.runtime)
self.logger.debug("Namespace %s" % self.namespace)

Expand All @@ -119,33 +128,43 @@ def reset_cache_time(self) -> None:

def reset_events(self) -> None:
if self._local_only:
assert(self._scout)
assert self._scout
typecast(LocalScout, self._scout).reset_events()

def __str__(self) -> str:
return ("%s: %s" % ("OK" if self._scout else "??",
self._scout_error if self._scout_error else "OK"))
return "%s: %s" % (
"OK" if self._scout else "??",
self._scout_error if self._scout_error else "OK",
)

@property
def scout(self) -> Optional[Union[Scout, LocalScout]]:
if not self._scout:
if self._local_only:
self._scout = LocalScout(logger=self.logger,
app=self.app, version=self.version, install_id=self.install_id)
self._scout = LocalScout(
logger=self.logger,
app=self.app,
version=self.version,
install_id=self.install_id,
)
self.logger.debug("LocalScout initialized")
else:
try:
self._scout = Scout(app=self.app, version=self.version, install_id=self.install_id)
self._scout = Scout(
app=self.app, version=self.version, install_id=self.install_id
)
self._scout_error = None
self.logger.debug("Scout connection established")
except OSError as e:
self._scout = None
self._scout_error = str(e)
self.logger.debug("Scout connection failed, will retry later: %s" % self._scout_error)
self.logger.debug(
"Scout connection failed, will retry later: %s" % self._scout_error
)

return self._scout

def report(self, force_result: Optional[dict]=None, no_cache=False, **kwargs) -> dict:
def report(self, force_result: Optional[dict] = None, no_cache=False, **kwargs) -> dict:
_notices: List[ScoutNotice] = []

# Silly, right?
Expand All @@ -163,11 +182,11 @@ def report(self, force_result: Optional[dict]=None, no_cache=False, **kwargs) ->
result_was_cached: bool = False

if not result:
if 'runtime' not in kwargs:
kwargs['runtime'] = self.runtime
if "runtime" not in kwargs:
kwargs["runtime"] = self.runtime

if 'commit' not in kwargs:
kwargs['commit'] = Commit
if "commit" not in kwargs:
kwargs["commit"] = Commit

# How long since the last Scout update? If it's been more than an hour,
# check Scout again.
Expand All @@ -179,7 +198,7 @@ def report(self, force_result: Optional[dict]=None, no_cache=False, **kwargs) ->
if use_cache:
if self._last_update:
since_last_update = now - typecast(datetime.datetime, self._last_update)
needs_update = (since_last_update > self._update_frequency)
needs_update = since_last_update > self._update_frequency

if needs_update:
if self.scout:
Expand All @@ -188,79 +207,88 @@ def report(self, force_result: Optional[dict]=None, no_cache=False, **kwargs) ->
self._last_update = now
self._last_result = dict(**typecast(dict, result)) if result else None
else:
result = { "scout": "unavailable: %s" % self._scout_error }
_notices.append({ "level": "DEBUG",
"message": "scout temporarily unavailable: %s" % self._scout_error })
result = {"scout": "unavailable: %s" % self._scout_error}
_notices.append(
{
"level": "DEBUG",
"message": "scout temporarily unavailable: %s" % self._scout_error,
}
)

# Whether we could talk to Scout or not, update the timestamp so we don't
# try again too soon.
result_timestamp = datetime.datetime.now()
else:
_notices.append({ "level": "DEBUG", "message": "Returning cached result" })
_notices.append({"level": "DEBUG", "message": "Returning cached result"})
result = dict(**typecast(dict, self._last_result)) if self._last_result else None
result_was_cached = True

# We can't get here unless self._last_update is set.
result_timestamp = typecast(datetime.datetime, self._last_update)
else:
_notices.append({ "level": "INFO", "message": "Returning forced Scout result" })
_notices.append({"level": "INFO", "message": "Returning forced Scout result"})
result_timestamp = datetime.datetime.now()

if not self.semver:
_notices.append({
"level": "WARNING",
"message": "Ambassador has invalid version '%s'??!" % self.version
})
_notices.append(
{
"level": "WARNING",
"message": "Ambassador has invalid version '%s'??!" % self.version,
}
)

if result:
result['cached'] = result_was_cached
result["cached"] = result_was_cached
else:
result = { 'cached': False }
result = {"cached": False}

result['timestamp'] = result_timestamp.timestamp()
result["timestamp"] = result_timestamp.timestamp()

# Do version & notices stuff.
if 'latest_version' in result:
latest_version = result['latest_version']
if "latest_version" in result:
latest_version = result["latest_version"]
latest_semver = self.get_semver(latest_version)

if latest_semver:
self._latest_version = latest_version
self._latest_semver = latest_semver
else:
_notices.append({
"level": "WARNING",
"message": "Scout returned invalid version '%s'??!" % latest_version
})

if (self._latest_semver and ((not self.semver) or
(self._latest_semver > self.semver))):
_notices.append({
"level": "INFO",
"message": "Upgrade available! to Ambassador version %s" % self._latest_semver
})

if 'notices' in result:
rnotices = typecast(List[Union[str, ScoutNotice]], result['notices'])
_notices.append(
{
"level": "WARNING",
"message": "Scout returned invalid version '%s'??!" % latest_version,
}
)

if self._latest_semver and ((not self.semver) or (self._latest_semver > self.semver)):
_notices.append(
{
"level": "INFO",
"message": "Upgrade available! to Ambassador version %s" % self._latest_semver,
}
)

if "notices" in result:
rnotices = typecast(List[Union[str, ScoutNotice]], result["notices"])

for notice in rnotices:
if isinstance(notice, str):
_notices.append({ "level": "WARNING", "message": notice })
_notices.append({"level": "WARNING", "message": notice})
elif isinstance(notice, dict):
lvl = notice.get('level', 'WARNING').upper()
msg = notice.get('message', None)
lvl = notice.get("level", "WARNING").upper()
msg = notice.get("message", None)

if msg:
_notices.append({ "level": lvl, "message": msg })
_notices.append({"level": lvl, "message": msg})
else:
_notices.append({ "level": "WARNING", "message": dump_json(notice) })
_notices.append({"level": "WARNING", "message": dump_json(notice)})

self._notices = _notices

if self._notices:
result['notices'] = self._notices
result["notices"] = self._notices
else:
result.pop('notices', None)
result.pop("notices", None)

return result

Expand Down
17 changes: 8 additions & 9 deletions python/ambassador/cache.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import logging


class Cacheable(dict):
"""
A dictionary that is specifically cacheable, by way of its added
Expand All @@ -27,7 +28,7 @@ def cache_key(self, cache_key: str) -> None:
CacheLink = Set[str]


class Cache():
class Cache:
"""
A cache of Cacheables, supporting add/delete/fetch and also linking
an owning Cacheable to an owned Cacheable. Deletion is cascaded: if you
Expand Down Expand Up @@ -55,8 +56,7 @@ def reset_stats(self) -> None:
def fn_name(fn: Optional[Callable]) -> str:
return fn.__name__ if (fn and fn.__name__) else "-none-"

def add(self, rsrc: Cacheable,
on_delete: Optional[DeletionHandler]=None) -> None:
def add(self, rsrc: Cacheable, on_delete: Optional[DeletionHandler] = None) -> None:
"""
Adds an entry to the cache, if it's not already present. If
on_delete is not None, it will called when rsrc is removed from
Expand Down Expand Up @@ -105,7 +105,7 @@ def link(self, owner: Cacheable, owned: Cacheable) -> None:
self.logger.debug(f"CACHE: linking {owner_key} -> {owned_key}")

links = self.links.setdefault(owner_key, set())
links.update([ owned_key ])
links.update([owned_key])

def invalidate(self, key: str) -> None:
"""
Expand All @@ -126,7 +126,7 @@ def invalidate(self, key: str) -> None:

self.invalidate_calls += 1

worklist = [ key ]
worklist = [key]

# Under the hood, "invalidating" something from this cache is really
# deleting it, so we'll use "to_delete" for the set of things we're going
Expand Down Expand Up @@ -167,10 +167,10 @@ def invalidate(self, key: str) -> None:
self.logger.debug(f"CACHE: DEL {key}: smiting!")

self.invalidated_objects += 1
del(self.cache[key])
del self.cache[key]

if key in self.links:
del(self.links[key])
del self.links[key]

rsrc, on_delete = rdh

Expand Down Expand Up @@ -245,8 +245,7 @@ def __init__(self, logger: logging.Logger) -> None:
self.reset_stats()
pass

def add(self, rsrc: Cacheable,
on_delete: Optional[DeletionHandler]=None) -> None:
def add(self, rsrc: Cacheable, on_delete: Optional[DeletionHandler] = None) -> None:
pass

def link(self, owner: Cacheable, owned: Cacheable) -> None:
Expand Down
Loading