Skip to content
Closed
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
15 changes: 8 additions & 7 deletions tools/stats/upload_stats_lib.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,14 +104,16 @@ def download_s3_artifacts(
paths = []
for obj in objs:
object_name = Path(obj.key).name
# target an artifact for a specific job_id if provided, otherwise skip the download.
if job_id is not None and str(job_id) not in object_name:
continue
found_one = True
p = Path(Path(obj.key).name)
print(f"Downloading {p}")
s3_resource = get_s3_resource()
bucket_obj = s3_resource.Bucket(GHA_ARTIFACTS_BUCKET)
obj_data = bucket_obj.Object(obj.key).get()
with open(p, "wb") as f:
f.write(obj.get()["Body"].read())
f.write(obj_data["Body"].read())
paths.append(p)

if not found_one:
Comment on lines 104 to 119

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Correctness: 🟠 [LangGraph v3] The s3_resource and bucket_obj are re-initialized inside the loop for each object, which is unnecessary and inefficient. Move the initialization outside the loop to improve performance.

📝 Committable Code Suggestion

‼️ Ensure you review the code suggestion before committing it to the branch. Make sure it replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
paths = []
for obj in objs:
object_name = Path(obj.key).name
# target an artifact for a specific job_id if provided, otherwise skip the download.
if job_id is not None and str(job_id) not in object_name:
continue
found_one = True
p = Path(Path(obj.key).name)
print(f"Downloading {p}")
s3_resource = get_s3_resource()
bucket_obj = s3_resource.Bucket(GHA_ARTIFACTS_BUCKET)
obj_data = bucket_obj.Object(obj.key).get()
with open(p, "wb") as f:
f.write(obj.get()["Body"].read())
f.write(obj_data["Body"].read())
paths.append(p)
if not found_one:
found_one = False
paths = []
s3_resource = get_s3_resource()
bucket_obj = s3_resource.Bucket(GHA_ARTIFACTS_BUCKET)
for obj in objs:
object_name = Path(obj.key).name
if job_id is not None and str(job_id) not in object_name:
continue
found_one = True
p = Path(Path(obj.key).name)
print(f"Downloading {p}")
obj_data = bucket_obj.Object(obj.key).get()
with open(p, "wb") as f:
f.write(obj_data["Body"].read())
paths.append(p)
if not found_one:

Expand Down Expand Up @@ -161,10 +163,8 @@ def upload_to_s3(
json.dump(doc, body)
body.write("\n")

get_s3_resource().Object(
f"{bucket_name}",
f"{key}",
).put(
s3_obj = get_s3_resource().Object(f"{bucket_name}", f"{key}")
s3_obj.put(
Body=gzip.compress(body.getvalue().encode()),
ContentEncoding="gzip",
ContentType="application/json",
Comment on lines 163 to 170

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Correctness: 🟠 [LangGraph v3] Using gzip.compress on StringIO content is inefficient for large data. Use BytesIO to avoid encoding/decoding overhead.

📝 Committable Code Suggestion

‼️ Ensure you review the code suggestion before committing it to the branch. Make sure it replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
json.dump(doc, body)
body.write("\n")
get_s3_resource().Object(
f"{bucket_name}",
f"{key}",
).put(
s3_obj = get_s3_resource().Object(f"{bucket_name}", f"{key}")
s3_obj.put(
Body=gzip.compress(body.getvalue().encode()),
ContentEncoding="gzip",
ContentType="application/json",
body = io.BytesIO()
for doc in docs:
body.write(json.dumps(doc).encode() + b"\n")
s3_obj = get_s3_resource().Object(f"{bucket_name}", f"{key}")
s3_obj.put(
Body=gzip.compress(body.getvalue()),
ContentEncoding="gzip",
ContentType="application/json",
)

Expand All @@ -186,7 +186,8 @@ def read_from_s3(
.get()["Body"]
.read()
)
results = gzip.decompress(body).decode().split("\n")
decompressed = gzip.decompress(body).decode()
results = decompressed.split("\n")
return [json.loads(result) for result in results if result]


Comment on lines 186 to 193

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Correctness: 🟠 [LangGraph v3] The variable decompressed is assigned but used only once. Directly use the result of gzip.decompress(body).decode() in the split method to simplify the code.

📝 Committable Code Suggestion

‼️ Ensure you review the code suggestion before committing it to the branch. Make sure it replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
.get()["Body"]
.read()
)
results = gzip.decompress(body).decode().split("\n")
decompressed = gzip.decompress(body).decode()
results = decompressed.split("\n")
return [json.loads(result) for result in results if result]
.get()["Body"]
.read()
)
results = gzip.decompress(body).decode().split("\n")
return [json.loads(result) for result in results if result]

Expand Down
2 changes: 2 additions & 0 deletions tools/stats/upload_test_stats.py
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,8 @@ def summarize_test_cases(test_cases: list[dict[str, Any]]) -> list[dict[str, Any
manually instead of using the `test-suite` XML tag because xmlrunner does
not produce reliable output for it.
"""

test_cases = sorted(test_cases, key=lambda x: (x.get("file", ""), x.get("classname", "")))

def get_key(test_case: dict[str, Any]) -> Any:
return (
Comment on lines 170 to 177

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Correctness: 🟠 [LangGraph v3] The sorting of test_cases is redundant as it is immediately followed by a grouping operation that does not rely on order. Remove the sorting to improve performance.

📝 Committable Code Suggestion

‼️ Ensure you review the code suggestion before committing it to the branch. Make sure it replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
manually instead of using the `test-suite` XML tag because xmlrunner does
not produce reliable output for it.
"""
test_cases = sorted(test_cases, key=lambda x: (x.get("file", ""), x.get("classname", "")))
def get_key(test_case: dict[str, Any]) -> Any:
return (
"""Group test cases by classname, file, and job_id. We perform the aggregation
manually instead of using the `test-suite` XML tag because xmlrunner does
not produce reliable output for it.
"""
def get_key(test_case: dict[str, Any]) -> Any:
return (

Expand Down