Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: removed the output field from the console log #21600

Merged

Conversation

rohitwaghchaure
Copy link
Contributor

Users on FC use the system console to test the SQL query of the report. While testing the SQL query using the system console, the system generates a console log containing the executed query and its output. The output field stores the actual data resulting from the query, which can be extensive. As users run the SQL query multiple times, the system creates a new console log record each time, which cause the database size limit issue.

As this logs are needed for the security reason and user won't able to delete it, it's better to not store the "Output" of the query in the database.

Screenshot 2023-07-05 at 10 29 16 PM

@rohitwaghchaure rohitwaghchaure requested review from a team and shariquerik and removed request for a team July 5, 2023 17:11
@github-actions github-actions bot added the add-test-cases Add test case to validate fix or enhancement label Jul 5, 2023
@codecov
Copy link

codecov bot commented Jul 5, 2023

Codecov Report

Merging #21600 (509c303) into develop (821f75c) will increase coverage by 0.01%.
The diff coverage is 75.00%.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #21600      +/-   ##
===========================================
+ Coverage    63.89%   63.90%   +0.01%     
===========================================
  Files          765      765              
  Lines        69365    69384      +19     
  Branches      6276     6276              
===========================================
+ Hits         44318    44340      +22     
- Misses       21440    21480      +40     
+ Partials      3607     3564      -43     
Flag Coverage Δ
server 69.03% <100.00%> (+0.13%) ⬆️
server-ui 29.96% <0.00%> (-2.34%) ⬇️
ui-tests 51.61% <66.66%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

@phot0n
Copy link
Contributor

phot0n commented Jul 6, 2023

can we also add clear_old_logs in console log's controller? so that whoever wants to clear the logs can add the entry in Log Settings

@ankush ankush merged commit 84c5e55 into frappe:develop Jul 11, 2023
23 checks passed
@ankush ankush removed the add-test-cases Add test case to validate fix or enhancement label Jul 11, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 27, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport version-14-hotfix backport to version 14
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants