2nd attempt at adding json exporting!#24
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds JSON exporting functionality to the query tracking system, allowing log data to be written to a file if configured.
- Introduces an export_log method in the core tracking class that exports log data as JSON.
- Updates the configuration to include export_type and export_path options for JSON export.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| pyquerytracker/core.py | Adds JSON export functionality in the log tracking workflow. |
| pyquerytracker/config.py | Introduces configuration settings for JSON export. |
Comments suppressed due to low confidence (1)
pyquerytracker/core.py:102
- Consider adding error handling for json.dump in case log_data includes non-serializable objects. This ensures that JSON export failures are caught and handled appropriately.
except Exception as e:
|
@ArchooD2, can you add a docstring or suppress the lint issue ? Also some test cases, once you are done. |
|
adding test cases |
|
There is another conflict. Will test it once you are done. |
…for JSON log export
|
Cool, added test case and reformatted with black and pylint testing 👍 Let me know if anything needs changing |
|
There are 2 files that have conflicts, can you resolve them? @ArchooD2 |
|
Is that fully fixed? |
| export_path = tmp_path / "log.json" | ||
|
|
||
| # Monkeypatch the config to enable JSON export | ||
| config = get_config() |
There was a problem hiding this comment.
We won't be using get_config, we will be using configure, look at examples/core/with_config.py, can you modify the config.py also?
Moved json export testing to new file Changed get_config patch in test to configure() call No longer writes to os.path.dirname(None) (disables JsonExporter class entirely if None path supplied) File writes should now be thread-safe Changed export logic to use pathlib rather than os
…ort (requested changes)
|
Thanks @ArchooD2, for you patience. |
|
Can you cherry-pick these commits by forking the repo and contributing? 
|
|
@ArchooD2, closing this PR. |
Hope this looks right, still haven't added tests in the suite.