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

Re-attempt censorship settings on query #75

Merged
merged 5 commits into from
Mar 27, 2024
Merged
Show file tree
Hide file tree
Changes from 3 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
4 changes: 2 additions & 2 deletions bento_beacon/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
from .utils.beacon_request import save_request_data, validate_request, verify_permissions
from .utils.beacon_response import init_response_data
from .utils.katsu_utils import katsu_censorship_settings
from .utils.censorship import set_censorship_settings

REQUEST_SPEC_RELATIVE_PATH = "beacon-v2/framework/json/requests/"
BEACON_MODELS = ["analyses", "biosamples", "cohorts", "datasets", "individuals", "runs", "variants"]
Expand Down Expand Up @@ -92,8 +93,7 @@
f"retrieved censorship params: max_filter {max_filters}, count_threshold: {count_threshold}")

# save even if None
current_app.config["MAX_FILTERS"] = max_filters
current_app.config["COUNT_THRESHOLD"] = count_threshold
set_censorship_settings(max_filters, count_threshold)


@app.before_request
Expand Down
2 changes: 1 addition & 1 deletion bento_beacon/config_files/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ class Config:
"schema": "phenopackets v1"
}

MAX_RETRIES_FOR_CENSORSHIP_PARAMS = 5
MAX_RETRIES_FOR_CENSORSHIP_PARAMS = 2
# -------------------
# gohan

Expand Down
35 changes: 29 additions & 6 deletions bento_beacon/utils/censorship.py
Original file line number Diff line number Diff line change
@@ -1,17 +1,42 @@
from flask import current_app, g
from .exceptions import APIException, InvalidQuery
from .katsu_utils import katsu_censorship_settings


MESSAGE_FOR_CENSORED_QUERY_WITH_NO_RESULTS = "No results. Either none were found, or the query produced results numbering at or below the threshold for censorship."


def set_censorship_settings(max_filters, count_threshold):
current_app.config["MAX_FILTERS"] = max_filters
current_app.config["COUNT_THRESHOLD"] = count_threshold


# saves settings to config as a side effect
def censorship_retry():
Copy link
Member

Choose a reason for hiding this comment

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

would be nice to have return type hints on this function and the two below (tuple[int | None, int | None], int | None), but otherwise this PR looks good

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks... I've started adding these incrementally where they're helpful

max_filters, count_threshold = katsu_censorship_settings()
if max_filters is None or count_threshold is None:
raise APIException(message="error reading censorship settings from katsu: "
+ f"max_filters: {max_filters}, count_threshold: {count_threshold}")

set_censorship_settings(max_filters, count_threshold)
return max_filters, count_threshold


def threshold_retry():
_, count_threshold = censorship_retry()
return count_threshold


def max_filters_retry():
max_filters, _ = censorship_retry()
return max_filters


def get_censorship_threshold():
if g.permission_query_data:
return 0
threshold = current_app.config["COUNT_THRESHOLD"]
if threshold is None:
raise APIException(message="unable to retrieve 'count_threshold' censorship parameter from katsu")
return threshold
return threshold if threshold is not None else threshold_retry()


def censored_count(count):
Expand All @@ -24,9 +49,7 @@ def censored_count(count):
# we don't have the same option of returning zero here
def get_max_filters():
max_filters = current_app.config["MAX_FILTERS"]
if max_filters is None:
raise APIException(message="unable to retrieve 'max_query_parameters' censorship setting from katsu")
return max_filters
return max_filters if max_filters is not None else max_filters_retry()


# ugly side-effect code, but keeps censorship code together
Expand Down