Skip to content

Commit

Permalink
Merge pull request emissary-ingress#4313 from emissary-ingress/lukesh…
Browse files Browse the repository at this point in the history 
…u/black

Add the `black` Python code formatter

It's silly how many code review comments are about formatting of
Python code.  Mechanical things like that are the job of computers,
not human reviewers.  For many years, there were many competing Python
code formatters.  However, in the last couple of years, the Python
community has come to a realization that the Go community realized
from the get-go: Agreement on the formatter is more important than the
formatting itself.  That even if the formatting isn't your favorite,
that not having to have discussions about formatting is even better.
So the Python community has standardized on the `black` formatter.

So adopt `black` the way we've adopted `gofmt`!

- The first 2 commits are addressing things to get them ready for
  black
- The 3rd commit is adding black in to our tooling, but not yet
  committing the results of running black
- The final commit is just running `make format/black` (running just
  `make format` would do this and a few other things too)

That last commit, while being machine-generated, is probably still
worthy of decent scrutiny, to make sure it didn't move around a
comment to where it's not near the code it's describing, or something
funny like that.

I considered turning parts of black off, and only re-formatting things
incrementally over time, so as to avoid merge-conflicts with current
in-flight Python work.  While tempting, there's still no good time to
turn each thing on, so I figure it's better to just bite the bullet
and get it over with rather than drawing it out over months.

Signed-off-by: Luke Shumaker <[email protected]>
  • Loading branch information
@LukeShu
LukeShu authored Jun 30, 2022
2 parents d678c14 + e110ebf commit 327dca4
Show file tree
Hide file tree
Showing 178 changed files with 15,078 additions and 9,366 deletions.
11 changes: 11 additions & 0 deletions build-aux/lint.mk
View file
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
View file
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
View file
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
View file
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
View file
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

0 comments on commit 327dca4

Please sign in to comment.