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

Applying Filters to the Telco Table #157

Open
wants to merge 12 commits into
base: revamp
Choose a base branch
from

Conversation

MVarshini
Copy link
Collaborator

Type of change

  • Refactor
  • New feature
  • Bug fix
  • Optimization
  • Documentation Update

Description

PANDA-700

This PR filters the data of Telco table

Related Tickets & Documents

  • Related Issue #
  • Closes #

Checklist before requesting a review

  • I have performed a self-review of my code.
  • If it is a core feature, I have added thorough tests.

Testing

  • Please describe the System Under Test.
  • Please provide detailed steps to perform tests related to this code change.
  • How were the fix/results from this change verified? Please provide relevant screenshots or results.

Comment on lines +133 to +144
test_type_filter = " OR ".join(
['test_type="{}"'.format(test_type) for test_type in test_types]
)
searchList = test_type_filter
if filter:
filter_dict = utils.get_dict_from_qs(filter)
searchQuery = utils.construct_query(filter_dict)
if "benchmark" in filter_dict:
searchList = searchQuery
else:
searchList = searchQuery + " " + test_type_filter

Copy link
Collaborator

Choose a reason for hiding this comment

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

This seems to be repeated between getFilterData and getData ... can it be factored out into a helper function?

k = constants.FIELDS_FILTER_DICT[key]
if len(values) > 1:
or_clause = " or ".join([f'{k}="{value}"' for value in values])
query_parts.append(f"{or_clause}")
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is really just query_parts.append(or_clause) 😆

for key, values in filter_dict.items():
k = constants.FIELDS_FILTER_DICT[key]
if len(values) > 1:
or_clause = " or ".join([f'{k}="{value}"' for value in values])
Copy link
Collaborator

Choose a reason for hiding this comment

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

Elsewhere you capitalized the Splunk keyword OR. It'd be a bit easier to read if you're consistent about such conventions. The all-caps does make it stand out, and is probably worthwhile.


try:
# Run the job and retrieve results
job = await self.service.jobs.create(
job = self.service.jobs.create(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Above where you removed the await, you replaced it with a not job.is_done() busy loop. I think we discussed a similar change earlier, though I don't remember exactly why it was better ... but more importantly you don't seem to be doing anything here to wait for job before referencing the results at line 134 ... won't that fail?

@@ -40,58 +45,71 @@ async def getData(
"latest_time": "{}T23:59:59".format(end_datetime.strftime("%Y-%m-%d")),
"output_mode": "json",
}
searchList = " OR ".join(
test_type_filter = " OR ".join(
Copy link
Collaborator

Choose a reason for hiding this comment

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

I shouldn't be commenting on lines you didn't change, but in other places you've changed the awkward old str.format() substitutions to more readable and modern f-strings -- that'd look better here, too. 😁

Comment on lines 65 to 107
mapped_list = []
for each_response in response["data"]:
end_timestamp = int(each_response["timestamp"])
test_data = each_response["data"]
threshold = await telcoGraphs.process_json(test_data, True)
hash_digest, encrypted_data = hasher.hash_encrypt_json(each_response)
execution_time_seconds = test_type_execution_times.get(
test_data["test_type"], 0
)
start_timestamp = end_timestamp - execution_time_seconds
start_time_utc = datetime.fromtimestamp(start_timestamp, tz=timezone.utc)
end_time_utc = datetime.fromtimestamp(end_timestamp, tz=timezone.utc)
kernel = test_data["kernel"] if "kernel" in test_data else "Undefined"

mapped_list.append(
{
"uuid": hash_digest,
"encryptedData": encrypted_data.decode("utf-8"),
"ciSystem": "Jenkins",
"benchmark": test_data["test_type"],
"kernel": kernel,
"shortVersion": test_data["ocp_version"],
"ocpVersion": test_data["ocp_build"],
"releaseStream": utils.getReleaseStream(
{"releaseStream": test_data["ocp_build"]}
),
"nodeName": test_data["node_name"],
"cpu": test_data["cpu"],
"formal": test_data["formal"],
"startDate": str(start_time_utc),
"endDate": str(end_time_utc),
"buildUrl": jenkins_url
+ "/"
+ str(test_data["cluster_artifacts"]["ref"]["jenkins_build"]),
"jobStatus": "failure" if (threshold != 0) else "success",
"jobDuration": execution_time_seconds,
}
)
if response:
for each_response in response["data"]:
end_timestamp = int(each_response["timestamp"])
test_data = each_response["data"]
threshold = await telcoGraphs.process_json(test_data, True)
hash_digest, encrypted_data = hasher.hash_encrypt_json(each_response)
execution_time_seconds = test_type_execution_times.get(
test_data["test_type"], 0
)
start_timestamp = end_timestamp - execution_time_seconds
start_time_utc = datetime.fromtimestamp(start_timestamp, tz=timezone.utc)
end_time_utc = datetime.fromtimestamp(end_timestamp, tz=timezone.utc)
kernel = test_data["kernel"] if "kernel" in test_data else "Undefined"

mapped_list.append(
{
"uuid": hash_digest,
"encryptedData": encrypted_data.decode("utf-8"),
"ciSystem": "Jenkins",
"benchmark": test_data["test_type"],
"kernel": kernel,
"shortVersion": test_data["ocp_version"],
"ocpVersion": test_data["ocp_build"],
"releaseStream": utils.getReleaseStream(
{"releaseStream": test_data["ocp_build"]}
),
"nodeName": test_data["node_name"],
"cpu": test_data["cpu"],
"formal": test_data["formal"],
"startDate": str(start_time_utc),
"endDate": str(end_time_utc),
"buildUrl": jenkins_url
+ "/"
+ str(test_data["cluster_artifacts"]["ref"]["jenkins_build"]),
"jobStatus": "failure" if (threshold != 0) else "success",
"jobDuration": execution_time_seconds,
}
)

jobs = pd.json_normalize(mapped_list)

return {"data": jobs, "total": response["total"]}
Copy link
Member

Choose a reason for hiding this comment

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

You added an if response check, but you then use the response value afterwards, not conditioned by it. What situation does this happen, and why doesn't it also apply to the "total" key/value?
If you want to do something else in that situation, you can add a condition for the not response, and exit early.

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.

3 participants