Skip to content

Json Exporter (#9)#36

Closed
ArchooD2 wants to merge 22 commits into
MuddyHope:mainfrom
ArchooD2:cherry-picked-json-exporter
Closed

Json Exporter (#9)#36
ArchooD2 wants to merge 22 commits into
MuddyHope:mainfrom
ArchooD2:cherry-picked-json-exporter

Conversation

@ArchooD2

@ArchooD2 ArchooD2 commented Jun 17, 2025

Copy link
Copy Markdown
Collaborator

Should be right, cherrypicked as I was expected to, I hope. (#9)

Copilot AI review requested due to automatic review settings June 17, 2025 00:44

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces a JSON exporter for query tracking logs along with associated tests and configuration updates. Key changes include:

  • Integration of a new JsonExporter class in pyquerytracker for threadsafe JSON log exporting.
  • Addition of tests in tests/exporter/test_json_exporter.py to validate the JSON export functionality.
  • Updates in core and configuration files to support JSON export and provide a usage example.

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Show a summary per file
File Description
tests/test_core.py Duplicate import statements need cleanup.
tests/exporter/test_json_exporter.py New test coverage for JSON exporting functionality.
pyquerytracker/exporter/json_exporter.py New exporter class for JSON logs.
pyquerytracker/exporter/init.py Updated export package initialization.
pyquerytracker/core.py Integrated JsonExporter but missing log_data initialization.
pyquerytracker/config.py Updated configuration docstrings for export settings.
examples/exporter/json_exporter.py Usage example for JSON exporter functionality.
Comments suppressed due to low confidence (2)

tests/test_core.py:5

  • Duplicate import of 'pytest' detected; consider removing the redundant import to maintain cleaner code.
import pytest

pyquerytracker/core.py:79

  • The variable 'log_data' is referenced in the successful execution branch but has not been initialized; consider defining 'log_data' with the appropriate log details before its use.
extra=log_data,

@ArchooD2

Copy link
Copy Markdown
Collaborator Author

oh god what did i break now

@ArchooD2

Copy link
Copy Markdown
Collaborator Author

MuddyHope please help, I thought I did it the right way

@MuddyHope

MuddyHope commented Jun 17, 2025 via email

Copy link
Copy Markdown
Owner

@ArchooD2

Copy link
Copy Markdown
Collaborator Author

I believe so for both cases. I can try again, but I think I did it right. 1 moment

@ArchooD2 ArchooD2 closed this Jun 17, 2025
@ArchooD2 ArchooD2 force-pushed the cherry-picked-json-exporter branch from bf2d19c to c3d2ff3 Compare June 17, 2025 01:00
@ArchooD2

Copy link
Copy Markdown
Collaborator Author

refixing this and then reopening.

PJSans and others added 7 commits June 16, 2025 19:01
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
@ArchooD2

Copy link
Copy Markdown
Collaborator Author

Okay, should be fixed.

@ArchooD2 ArchooD2 reopened this Jun 17, 2025
@MuddyHope

MuddyHope commented Jun 17, 2025 via email

Copy link
Copy Markdown
Owner

@ArchooD2

Copy link
Copy Markdown
Collaborator Author

Found a minor bug with the json formatting

with _lock, self._path.open("a", encoding="utf-8") as f:
json.dump(record, f)
f.write("\n")
f.write(",\n")

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

The opening and closing brackets ?? []??

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

resolved in newest push

…log export. Holy hell, making this use json instead of jsonl was tougher than expected.
@ArchooD2

ArchooD2 commented Jun 17, 2025

Copy link
Copy Markdown
Collaborator Author

This is embarassing...

Edit: ARE YOU KIDDING ME LINT

@ArchooD2 ArchooD2 requested a review from MuddyHope June 17, 2025 01:51
@MuddyHope

Copy link
Copy Markdown
Owner

Hi @ArchooD2, can you run the examples/exporter/json_exporter.py function once, the slow_function and faulty_function aren't appearing on the json file. Can you add these also in the test module?

@MuddyHope MuddyHope linked an issue Jun 17, 2025 that may be closed by this pull request
@ArchooD2

Copy link
Copy Markdown
Collaborator Author

I'm gonna have words to say with an inanimate github workflow (just the linter one, I'm mildly displeased with how often I'm forgetting to lint)

@ArchooD2

Copy link
Copy Markdown
Collaborator Author

Seems to be working fine now @MuddyHope

@MuddyHope

MuddyHope commented Jun 17, 2025

Copy link
Copy Markdown
Owner

Hi @ArchooD2, the previous commits were good, you changed after my last review. Also, the flush wasn't so necessary. If you can remove it, that would be good. When I ran the examples/exporter files twice, there was no appending. Which should be necessary to save these queries.

Take your time and test it once, with the existing features and submit the PR. Hoping to see these changes.

@ArchooD2 ArchooD2 force-pushed the cherry-picked-json-exporter branch from a331b2c to 5e51a84 Compare June 17, 2025 06:15
PJSans added 3 commits June 17, 2025 00:20
…es. Added sanity checks in test_json_exporter for use in the event of further bugs.

@MuddyHope MuddyHope left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

If you are using AI to write your code, it's better to give it more context; try using Cursor.

Comment thread pyquerytracker/core.py Outdated
def flush_exported_logs() -> None:
"""
Manually flushes the JSON exporter buffer to disk.
Useful for ensuring log data is written in real-time or during tests.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Can all these exporter functions be put in utils?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

✔️

Comment thread pyquerytracker/config.py Outdated
@@ -1,3 +1,5 @@
# pyquerytracker/config.py

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Can you remove this line?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

✔️

Comment thread tests/test_core.py Outdated
import logging
import pytest
from pyquerytracker import TrackQuery
import pytest

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Why two imports here?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Fixed, accidentally added. ✔️

Comment thread pyquerytracker/core.py
self.config.slow_log_level,
f"{class_name}.{func.__name__} -> Slow execution: took %.2fms",
"%s.%s → Slow execution: took %.2fms",
class_name or "<module>",

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

what is this ?

Comment thread pyquerytracker/core.py Outdated
class _ExporterManager:
_instance: JsonExporter | None = None

@classmethod

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Check if it's really necessary, if it's really necessary, keep it in the utils file.

PJSans added 2 commits June 17, 2025 18:40
…tions to pyquerytracker/exporter/utils.py for better structure
@MuddyHope

MuddyHope commented Jun 18, 2025

Copy link
Copy Markdown
Owner

Usemake lint to refractor and checking linting issues.

@ArchooD2

ArchooD2 commented Jun 18, 2025

Copy link
Copy Markdown
Collaborator Author

make lint? 1 sec.

@ArchooD2

Copy link
Copy Markdown
Collaborator Author

Realized way too late that my choco wasn't installed properly, so make just didn't work...

@MuddyHope

MuddyHope commented Jun 18, 2025

Copy link
Copy Markdown
Owner

Are you done? @ArchooD2 ?
If not, can you remove the flush, we don't want to delete the existing one, it should append to the file.
Right now it is flushing the data.

@ArchooD2

Copy link
Copy Markdown
Collaborator Author

Updated the flush logic so that it appends! I should note that examples/exporter/json_exporter.py is much faster

@ArchooD2 ArchooD2 requested a review from MuddyHope June 19, 2025 01:11
@ArchooD2

Copy link
Copy Markdown
Collaborator Author

I think I'm done, if there aren't any more problems.

@MuddyHope

Copy link
Copy Markdown
Owner

Hi @ArchooD2, there are some conflicts. There was a change made to the framework of the exporter class.

@ArchooD2

ArchooD2 commented Jun 19, 2025

Copy link
Copy Markdown
Collaborator Author

I'm sorry, but how many changes to main have there been while I was working on this?

FOLLOWUP, WHY DID IT NEVER TELL ME

ArchooD2 and others added 2 commits June 19, 2025 08:16
…??????????????????????????????????????????????????????????????????????????????? (Fix pytest fail and linting issues.)
@MuddyHope

Copy link
Copy Markdown
Owner

Hi @ArchooD2, can you make the changes in accordance with the framework. It should be easier now with having a JsonExporter as there is already a BaseClass that have you to inherit from.

@MuddyHope

Copy link
Copy Markdown
Owner

@ArchooD2, are you working on this?

@ArchooD2

Copy link
Copy Markdown
Collaborator Author

I think I need to take some time away for a little, please allow anyone else to make a json exporter for this project 👍

@ArchooD2 ArchooD2 closed this Jun 22, 2025
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.

Add JSON Exporter

3 participants