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

small cleanups in req logger #3281

Merged
merged 2 commits into from
Jun 11, 2021
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
8 changes: 4 additions & 4 deletions components/seldon-request-logger/app/default_logger.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ def metadata():
print('in metadata endpoint - this is for debugging')
print(request.args)
serving_engine = request.args.get('serving_engine','SeldonDeployment')
if serving_engine is None or not serving_engine:
if not serving_engine:
serving_engine = 'SeldonDeployment'

namespace = request.args.get('namespace','seldon')
Expand Down Expand Up @@ -145,7 +145,7 @@ def process_and_update_elastic_doc(

item_body[message_type]["instance"] = item

if type(elements) == type([]) and len(elements) >= num:
if type(elements) == type([]) and len(elements) > num:
item_body[message_type]["elements"] = elements[num]

item_request_id = build_request_id_batched(
Expand Down Expand Up @@ -477,7 +477,7 @@ def createElelmentsArray(X: np.ndarray, names: list, namespace_name, serving_eng
sys.stdout.flush()

results = None
if not metadata_schema or metadata_schema is None:
if not metadata_schema:
results = createElementsNoMetadata(X, names, results)
else:
results = createElementsWithMetadata(X, names, results, metadata_schema, message_type)
Expand Down Expand Up @@ -597,7 +597,7 @@ def mergeLinkedColumns(raw_list, metadata_dict):
def lookupValueWithMetadata(name, metadata_dict, raw_value):
metadata_elem = metadata_dict[name]

if metadata_elem is None:
if not metadata_elem:
return raw_value

#categorical currently only case where we replace value
Expand Down
20 changes: 10 additions & 10 deletions components/seldon-request-logger/app/log_helper.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ def get_max_payload_bytes(default_value):

def extract_request_id(headers):
request_id = headers.get(REQUEST_ID_HEADER_NAME)
if request_id is None:
if not request_id:
# TODO: need to fix this upstream - https://github.com/kubeflow/kfserving/pull/699/files#diff-de6e9737c409666fc6c48dbcb50363faR18
request_id = headers.get(CLOUD_EVENT_ID)
return request_id
Expand All @@ -51,22 +51,22 @@ def build_index_name(headers):
# otherwise create an index per deployment
# index_name = "inference-log-" + serving_engine(headers)
namespace = clean_header(NAMESPACE_HEADER_NAME, headers)
if namespace is None:
if not namespace:
index_name = index_name + "-unknown-namespace"
else:
index_name = index_name + "-" + namespace
inference_service_name = clean_header(INFERENCESERVICE_HEADER_NAME, headers)
# won't get inference service name for older kfserving versions i.e. prior to https://github.com/kubeflow/kfserving/pull/699/
if inference_service_name is None or not inference_service_name:
if not inference_service_name:
inference_service_name = clean_header(MODELID_HEADER_NAME, headers)

if inference_service_name is None:
if not inference_service_name:
index_name = index_name + "-unknown-inferenceservice"
else:
index_name = index_name + "-" + inference_service_name

endpoint_name = clean_header(ENDPOINT_HEADER_NAME, headers)
if endpoint_name is None:
if not endpoint_name:
index_name = index_name + "-unknown-endpoint"
else:
index_name = index_name + "-" + endpoint_name
Expand Down Expand Up @@ -111,14 +111,14 @@ def set_metadata(content, headers, message_type, request_id):

inference_service_name = content.get(INFERENCESERVICE_HEADER_NAME)
# kfserving won't set inferenceservice header
if inference_service_name is None or not inference_service_name:
if not inference_service_name:
content[INFERENCESERVICE_HEADER_NAME] = clean_header(
MODELID_HEADER_NAME, headers
)

if message_type == "request" or not "@timestamp" in content:
timestamp = headers.get(TIMESTAMP_HEADER_NAME)
if timestamp is None:
if not timestamp:
timestamp = datetime.datetime.now(datetime.timezone.utc).isoformat()
content["@timestamp"] = timestamp

Expand All @@ -134,17 +134,17 @@ def serving_engine(headers):
return "inferenceservice"

def get_header(header_name, headers):
if not headers.get(header_name) is None:
if headers.get(header_name):
return clean_header(header_name, headers)

def field_from_header(content, header_name, headers):
if not headers.get(header_name) is None:
if headers.get(header_name):
content[header_name] = clean_header(header_name, headers)


def clean_header(header_name, headers):
header_val = headers.get(header_name)
if not header_val is None:
if header_val:
header_val = header_val.translate({ord(c): None for c in '!@#$"<>/?'})
return header_val

Expand Down
10 changes: 5 additions & 5 deletions components/seldon-request-logger/app/log_mapping.py
Original file line number Diff line number Diff line change
Expand Up @@ -135,14 +135,14 @@ def init_api():
config.verify_ssl = False
os.environ["CURL_CA_BUNDLE"] = ""

if not config.auth_method or config.auth_method is None:
if not config.auth_method:
config.auth_method = 'password_grant'

if not config.host or config.host is None:
if not config.host:
print('No DEPLOY_API_HOST - will not look up metadata from Deploy')
return

if not config.oidc_server or config.oidc_server is None:
if not config.oidc_server:
print('No OIDC_PROVIDER - auth will not be used in connecting to metadata')
return

Expand Down Expand Up @@ -176,7 +176,6 @@ def fetch_metadata(namespace, serving_engine, inferenceservice_name, predictor_n
if not deployment_type:
print('unknown deployment type for '+namespace+' / '+inferenceservice_name)
print(deployment_type)
#TODO: should be sending deployment_type in call below but currently sdk says unexpected keyword argument

if metadata_api is None:
print('metadata service not configured')
Expand All @@ -185,7 +184,8 @@ def fetch_metadata(namespace, serving_engine, inferenceservice_name, predictor_n
# TODO: in next iteration will only need one lookup straight to model metadata
# was expcting to set deployment_type=serving_engine but deployment_type does not seem to be a param
runtime_metadata = metadata_api.model_metadata_service_list_runtime_metadata_for_model(
deployment_name=inferenceservice_name,deployment_namespace=namespace,predictor_name=predictor_name)
deployment_name=inferenceservice_name,deployment_namespace=namespace,
predictor_name=predictor_name,deployment_type=deployment_type)

if runtime_metadata is not None and runtime_metadata and \
runtime_metadata.runtime_metadata is not None and runtime_metadata.runtime_metadata:
Expand Down
2 changes: 1 addition & 1 deletion components/seldon-request-logger/requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,4 @@ dict_digger==0.2.1
seldon_core
elasticsearch==7.12.1
click==8.0.0a1
seldon-deploy-sdk==1.3.0.dev2
seldon-deploy-sdk==1.3.0.dev3