Performance improvements - #5
ShashankFC wants to merge 2 commits into
Conversation
Entelligence AI Vulnerability ScannerStatus: No security vulnerabilities found Your code passed our comprehensive security analysis. Analyzed 2 files in total |
Review Summary❌ Rejected Comments (1)
🏷️ Draft Comments (6)
|
WalkthroughThis PR focuses on code quality improvements in the stats upload tooling. The changes include stylistic refactoring in Changes
Sequence DiagramThis diagram shows the interactions between components: sequenceDiagram
participant Client as Client Code
participant S3Svc as S3 Resource Service
participant Bucket as S3 Bucket
participant S3Obj as S3 Object
participant FS as File System
Note over Client,FS: Artifact Download Flow (Change Block 1)
loop for each object in objs
Client->>Client: Check if job_id matches object_name
alt job_id matches or is None
Client->>S3Svc: get_s3_resource()
S3Svc-->>Client: s3_resource
Client->>Bucket: s3_resource.Bucket(GHA_ARTIFACTS_BUCKET)
Bucket-->>Client: bucket_obj
Client->>S3Obj: bucket_obj.Object(obj.key).get()
S3Obj-->>Client: obj_data
Client->>S3Obj: obj_data["Body"].read()
S3Obj-->>Client: file_content
Client->>FS: write(file_content)
FS-->>Client: success
else job_id doesn't match
Client->>Client: skip download
end
end
Note over Client,S3Obj: Artifact Upload Flow (Change Block 2)
Client->>Client: Prepare gzipped body content
Client->>S3Svc: get_s3_resource()
S3Svc-->>Client: s3_resource
Client->>S3Obj: s3_resource.Object(bucket_name, key)
S3Obj-->>Client: s3_obj
Client->>S3Obj: s3_obj.put(Body, ContentEncoding, ContentType)
S3Obj-->>Client: upload complete
Note over Client,S3Obj: Artifact Retrieval & Decompression (Change Block 3)
Client->>S3Obj: get()["Body"].read()
S3Obj-->>Client: compressed_body
Client->>Client: gzip.decompress(body).decode()
Client->>Client: decompressed.split("\n")
Client->>Client: Parse JSON results
🔗 Cross-Repository Impact AnalysisEnable automatic detection of breaking changes across your dependent repositories. → Set up now Learn more about Cross-Repository AnalysisWhat It Does
How to Enable
Benefits
Note for WindsurfPlease change the default marketplace provider to the following in the windsurf settings:Marketplace Extension Gallery Service URL: https://marketplace.visualstudio.com/_apis/public/gallery Marketplace Gallery Item URL: https://marketplace.visualstudio.com/items Entelligence.ai can learn from your feedback. Simply add 👍 / 👎 emojis to teach it your preferences. More shortcuts belowEmoji Descriptions:
Interact with the Bot:
Also you can trigger various commands with the bot by doing The current supported commands are
More commands to be added soon. |
Review Summary🏷️ Draft Comments (8)
|
WalkthroughThis PR focuses on code quality improvements in the stats upload tooling. The changes include stylistic refactoring in Changes
Sequence DiagramThis diagram shows the interactions between components: sequenceDiagram
participant Client as Client Code
participant S3Svc as S3 Resource Service
participant Bucket as S3 Bucket
participant S3Obj as S3 Object
participant FS as File System
Note over Client,FS: Artifact Download Flow (Change Block 1)
loop for each object in objs
Client->>Client: Check if job_id matches object_name
alt job_id matches or is None
Client->>S3Svc: get_s3_resource()
S3Svc-->>Client: s3_resource
Client->>Bucket: s3_resource.Bucket(GHA_ARTIFACTS_BUCKET)
Bucket-->>Client: bucket_obj
Client->>S3Obj: bucket_obj.Object(obj.key).get()
S3Obj-->>Client: obj_data
Client->>S3Obj: obj_data["Body"].read()
S3Obj-->>Client: file_content
Client->>FS: write(file_content)
FS-->>Client: success
end
end
Note over Client,S3Obj: Artifact Upload Flow (Change Block 2)
Client->>Client: Prepare gzipped body content
Client->>S3Svc: get_s3_resource()
S3Svc-->>Client: s3_resource
Client->>S3Obj: s3_resource.Object(bucket_name, key)
S3Obj-->>Client: s3_obj
Client->>S3Obj: s3_obj.put(Body, ContentEncoding, ContentType)
S3Obj-->>Client: upload complete
Note over Client,S3Obj: Artifact Retrieval & Decompression (Change Block 3)
Client->>S3Obj: get()["Body"].read()
S3Obj-->>Client: compressed_body
Client->>Client: gzip.decompress(body).decode()
Client->>Client: decompressed.split("\n")
Client->>Client: Parse JSON results
🔗 Cross-Repository Impact AnalysisEnable automatic detection of breaking changes across your dependent repositories. → Set up now Learn more about Cross-Repository AnalysisWhat It Does
How to Enable
Benefits
Note for WindsurfPlease change the default marketplace provider to the following in the windsurf settings:Marketplace Extension Gallery Service URL: https://marketplace.visualstudio.com/_apis/public/gallery Marketplace Gallery Item URL: https://marketplace.visualstudio.com/items Entelligence.ai can learn from your feedback. Simply add 👍 / 👎 emojis to teach it your preferences. More shortcuts belowEmoji Descriptions:
Interact with the Bot:
Also you can trigger various commands with the bot by doing The current supported commands are
More commands to be added soon. |
Review Summary❌ Rejected Comments (1)
🏷️ Draft Comments (5)
🔍 Comments beyond diff scope (1)
|
🔬 Multi-Approach Review SummaryThis PR was reviewed by 2 different approaches for comparison:
Total: 0 review comments Each comment is labeled with its source approach. This allows you to compare different AI review strategies. 🔒 Security Scan: Run once and shared across all approaches for efficiency. WalkthroughThis PR focuses on code quality improvements in the stats upload tooling. The changes include stylistic refactoring in Changes
Sequence DiagramThis diagram shows the interactions between components: sequenceDiagram
participant Client as Client Code
participant S3Svc as S3 Resource Service
participant Bucket as S3 Bucket
participant S3Obj as S3 Object
participant FS as File System
Note over Client,FS: Artifact Download Flow (Change Block 1)
loop for each object in objs
Client->>Client: Check if job_id matches object_name
alt job_id matches or is None
Client->>S3Svc: get_s3_resource()
S3Svc-->>Client: s3_resource
Client->>Bucket: s3_resource.Bucket(GHA_ARTIFACTS_BUCKET)
Bucket-->>Client: bucket_obj
Client->>S3Obj: bucket_obj.Object(obj.key).get()
S3Obj-->>Client: obj_data
Client->>S3Obj: obj_data["Body"].read()
S3Obj-->>Client: file_content
Client->>FS: write(file_content)
FS-->>Client: success
end
end
Note over Client,S3Obj: Artifact Upload Flow (Change Block 2)
Client->>Client: Prepare gzipped body content
Client->>S3Svc: get_s3_resource()
S3Svc-->>Client: s3_resource
Client->>S3Obj: s3_resource.Object(bucket_name, key)
S3Obj-->>Client: s3_obj
Client->>S3Obj: s3_obj.put(Body, ContentEncoding, ContentType)
S3Obj-->>Client: upload complete
Note over Client,S3Obj: Artifact Retrieval & Decompression (Change Block 3)
Client->>S3Obj: get()["Body"].read()
S3Obj-->>Client: compressed_body
Client->>Client: gzip.decompress(body).decode()
Client->>Client: decompressed.split("\n")
Client->>Client: Parse JSON results
🔒 Security Analysis
Note for WindsurfPlease change the default marketplace provider to the following in the windsurf settings:Marketplace Extension Gallery Service URL: https://marketplace.visualstudio.com/_apis/public/gallery Marketplace Gallery Item URL: https://marketplace.visualstudio.com/items Entelligence.ai can learn from your feedback. Simply add 👍 / 👎 emojis to teach it your preferences. More shortcuts belowEmoji Descriptions:
Interact with the Bot:
Also you can trigger various commands with the bot by doing The current supported commands are
More commands to be added soon. |
Review Summary❌ Rejected Comments (2)
🏷️ Draft Comments (5)
|
🔬 Multi-Approach Review SummaryThis PR was reviewed by 2 different approaches for comparison:
Total: 4 review comments Each comment is labeled with its source approach. This allows you to compare different AI review strategies. 🔒 Security Scan: Run once and shared across all approaches for efficiency. WalkthroughThis PR focuses on code quality improvements in the stats upload tooling. The changes include stylistic refactoring in Changes
Sequence DiagramThis diagram shows the interactions between components: sequenceDiagram
participant Client as Client Code
participant S3Svc as S3 Resource Service
participant Bucket as S3 Bucket
participant S3Obj as S3 Object
participant FS as File System
Note over Client,FS: Artifact Download Flow (Change Block 1)
loop for each object in objs
Client->>Client: Check if job_id matches object_name
alt job_id matches or is None
Client->>S3Svc: get_s3_resource()
S3Svc-->>Client: s3_resource
Client->>Bucket: s3_resource.Bucket(GHA_ARTIFACTS_BUCKET)
Bucket-->>Client: bucket_obj
Client->>S3Obj: bucket_obj.Object(obj.key).get()
S3Obj-->>Client: obj_data
Client->>S3Obj: obj_data["Body"].read()
S3Obj-->>Client: file_content
Client->>FS: write(file_content)
FS-->>Client: success
end
end
Note over Client,S3Obj: Artifact Upload Flow (Change Block 2)
Client->>Client: Prepare gzipped body content
Client->>S3Svc: get_s3_resource()
S3Svc-->>Client: s3_resource
Client->>S3Obj: s3_resource.Object(bucket_name, key)
S3Obj-->>Client: s3_obj
Client->>S3Obj: s3_obj.put(Body, ContentEncoding, ContentType)
S3Obj-->>Client: upload complete
Note over Client,S3Obj: Artifact Retrieval & Decompression (Change Block 3)
Client->>S3Obj: get()["Body"].read()
S3Obj-->>Client: compressed_body
Client->>Client: gzip.decompress(body).decode()
Client->>Client: decompressed.split("\n")
Client->>Client: Parse JSON results
🔒 Security Analysis
Note for WindsurfPlease change the default marketplace provider to the following in the windsurf settings:Marketplace Extension Gallery Service URL: https://marketplace.visualstudio.com/_apis/public/gallery Marketplace Gallery Item URL: https://marketplace.visualstudio.com/items Entelligence.ai can learn from your feedback. Simply add 👍 / 👎 emojis to teach it your preferences. More shortcuts belowEmoji Descriptions:
Interact with the Bot:
Also you can trigger various commands with the bot by doing The current supported commands are
More commands to be added soon. |
![entelligence-ai-pr-reviews[bot]](https://avatars.githubusercontent.com/in/932812?s=60&v=4){kind=small}
| 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: |
There was a problem hiding this comment.
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.
| 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: |
| 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", |
There was a problem hiding this comment.
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.
| 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", | |
| ) |
| .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] | ||
|
|
||
|
|
There was a problem hiding this comment.
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.
| .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] | |
| 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 ( |
There was a problem hiding this comment.
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.
| 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 ( |
2 participants
EntelligenceAI PR Summary
This PR improves code quality in stats upload tools through stylistic refactoring and deterministic test case ordering.
download_s3_artifacts(),upload_to_s3(), andread_from_s3()by extracting intermediate variablessummarize_test_cases()for consistent output