feat: Expose accumulated metrics #813
Open
+44
−8
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
📜 Docstring Coverage ReportRESULT: PASSED (minimum: 30.0%, actual: 75.6%) Detailed Coverage Report |
📦 Trivy Vulnerability Scan Results
Report Summary
Scan Result Details✅ No vulnerabilities found during the scan for |
📦 Trivy Secret Scan Results
Report Summary
Scan Result Details✅ No secrets found during the scan for |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Collaborator
☂️ Python Coverage
Overall Coverage
New FilesNo new covered files... Modified FilesNo covered modified files...
|
|
🛠 Full Test Coverage Report: https://k.atlan.dev/coverage/application-sdk/pr/813 |
|
🛠 Docs available at: https://k.atlan.dev/application-sdk/APP-8299 |
| ) | ||
| await transformed_output.write_daft_dataframe(transform_metadata) | ||
| return await transformed_output.get_statistics() | ||
| return await transformed_output.get_statistics(typename=typename) |
There was a problem hiding this comment.
Bug: Metadata Handling Flaw Breaks Data Transformation
The write_daft_dataframe call is outside the conditional block that assigns transform_metadata. When the dataframe is empty, transform_metadata will be undefined causing a NameError. Additionally, if multiple dataframes are processed and some are empty, the write operation will incorrectly reuse the previous non-empty transformation result.
There was a problem hiding this comment.
Bug: Update Test for File Handling Changes
The test expects file to be opened with mode "w" and path /test/path/statistics.json.ignore, but the implementation now uses mode "wb" and path /test/path/statistics/statistics.json.ignore. The test needs updating to match the new implementation.
tests/unit/outputs/test_output.py#L139-L140
application-sdk/tests/unit/outputs/test_output.py
Lines 139 to 140 in 7ecb605
| assert result is None | ||
| mock_logger.assert_called_once() | ||
| assert "Error writing statistics" in mock_logger.call_args[0][0] | ||
|
|
There was a problem hiding this comment.
Bug: Error handling mismatch breaks tests.
The test expects write_statistics to return None on error, but the implementation now raises exceptions instead (line 455 adds explicit raise). The test expects old swallowing behavior that no longer exists, causing the test to fail.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changelog
Additional context (e.g. screenshots, logs, links)
Checklist
Copyleft License Compliance
Note
Propagates typename into statistics and writes stats files under a new statistics/ directory, updating SQL transform to pass typename.
Output.get_statisticsnow forwardstypenametowrite_statistics.write_statistics(typename)addstypenameto payload, writes tostatistics/statistics.json.ignore(orstatistics-chunk-{chunk_start}.json.ignore), and uploads that path._infer_phase_from_pathutility.transform_datanow callstransformed_output.get_statistics(typename=typename)to ensuretypenameis captured in stats.tests/unit/outputs/test_output.pyaligned with statistics writing behavior.Written by Cursor Bugbot for commit 7ecb605. This will update automatically on new commits. Configure here.