Skip to content

Conversation

@janbritz
Copy link
Contributor

@janbritz janbritz commented Oct 30, 2025

Closes #191

Mit VAR: ${QPY_VAR} wird der Wert der Umgebungsvariable QPY_VAR gelesen und verwendet. Wenn man explizit ${QPY_VAR} als Wert übergeben will, müssen zwei Dollarzeichen verwendet werden: $${QPY_VAR}. Dieses Verhalten ähnelt dem von Docker und systemd.

Wollen wir weiterhin OPENBLAS_NUM_THREADS dem SubprocessWorker übergeben? Theoretisch kann ein Admin das jetzt selber einstellen.

@janbritz janbritz force-pushed the feat/request-env-vars branch 4 times, most recently from b717033 to 0cd274c Compare November 3, 2025 13:13
@janbritz janbritz force-pushed the feat/request-env-vars branch from 0cd274c to 5e1681c Compare November 3, 2025 13:16
@janbritz janbritz marked this pull request as ready for review November 3, 2025 13:16
@MartinGauk
Copy link
Contributor

Wollen wir weiterhin OPENBLAS_NUM_THREADS dem SubprocessWorker übergeben? Theoretisch kann ein Admin das jetzt selber einstellen.

Ja, aber ist schon angenehmer, wenn solche Standard-Sachen direkt definiert sind. Wenn wir die CPU-Nutzung per cgroups einschränken, sollten wir am besten den Wert auch dynamisch setzen.

@janbritz janbritz force-pushed the feat/request-env-vars branch from 648a3ee to 01c677e Compare November 4, 2025 12:35
@janbritz janbritz force-pushed the feat/request-env-vars branch from 01c677e to 4efec60 Compare November 4, 2025 12:37
@janbritz janbritz requested a review from MartinGauk November 4, 2025 12:37
)

ENVIRONMENT_VARIABLE_REGEX: Final[str] = r"[a-zA-Z_][a-zA-Z0-9_]*"
ENVIRONMENT_VARIABLE = Annotated[str, Field(pattern=f"^{ENVIRONMENT_VARIABLE_REGEX}$")]
Copy link
Member

Choose a reason for hiding this comment

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

Nicht eher EnvironmentVariable bzw. EnvironmentVariableName?

Comment on lines +39 to +67
class Selector[T: Selectable, V](ABC):
"""Provides helpful methods for getting package and request specific data."""

def __init__(self, selectables: list[T]):
self._cache: LRUCacheMemory[SelectorQuery, V] = LRUCacheMemory(max_size=128)
self._selectables = selectables

def _get_matching(self, query: SelectorQuery) -> T | None:
"""Gets the first matching selectable, if any.
It assumes that the selectables are ordered from least specific to most specific.
"""
for selectable in reversed(self._selectables):
if _is_matching(selectable.package_selector, query):
return selectable
return None

def get(self, query: SelectorQuery) -> V:
"""Gets the result of the query, which may be cached."""
if cached := self._cache.get(query):
return cached

result = self._get(query)
self._cache.put(query, result)

return result

@abstractmethod
def _get(self, query: SelectorQuery) -> V: ...
Copy link
Member

Choose a reason for hiding this comment

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

Ich denke diese Klasse vermischt ein paar Dinge:

  • get cached
  • _get_matching macht in 4 Zeilen das, was eigentlich im Klassennamen steht, und wählt den richtigen Eintrag aus den Selectables aus.
  • _get machen in den Impls ziemlich verschiedene Dinge

Ich würde vorschlagen, das etwas zu refactoren:

  • _get_matching wird eine Funktion (oder Selector eine nicht-abstrakte Klasse, die nur das macht)
  • PackagePermissionsHandler und EnvironmentVariablesHandler haben keine Superklasse, sondern verwenden die neue Funktion
  • Die _get werden in den jeweiligen Klassen public. Falls Caching doch nötig sein sollte, sollte das da passieren.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Umgebungsvariablen übergeben

4 participants