Show PostgreSQL Server Log. #3981 #7381
Closed
Conversation
This file contains 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
akshay-joshi
requested changes
Apr 17, 2024
There was a problem hiding this comment.
GUI Related:
- Encountering the error "cannot access the local variable 'final_cols' where it is not associated with a value." when clicking on 'Table Based logs'.
- I think we should shift the entire "Server activity" tab near to "System Statistics"?
- We should display all available log formats and enable only those specified in the 'log_destination'.
- 'View the active session details" and 'View the log details' not working properly, and controls are not visible and overlapped.
- Clicking on the search box of each column is inadvertently sorting the column, which should not occur.
- The toggle button separator should be visible when selecting 'Text'. However, there is no separator between JSON and CSV.
- Remove the colon ":" from "LOG:" and "DEBUG:" from the values under 'Error Severity'.
- Rename the 'Error Severity' column to 'Log Mode' or 'Log Type', or a similar descriptive term. I know the term is from PostgreSQL, but LOG, and DEBUG messages are not errors.
- Rename the label 'Table Based Logs' to 'Show logs in tabular format?' or a similar descriptive term.
- On clicking the 'Table Based Logs' label switch control toggles its state.
- Timestamp/Log prefix not visible in the 'Text' mode.
- We should read all the files. Needs to be confirmed.
- Unnessary verticle Scroll Bar is visible on the Dashboard. This is not because of this PR but can you please look into it?
- Logs are missing in the 'Table Based logs' as it is visible in the file-based. Please check once.
- Reduce the width of the 'Error Severity', and ''View the log details' columns (if possible)
Code Related:
- Add Documentation.
- JavaScript tests are failing.
- Fixed SonarQube issues that were newly added with this PR.
- Add proper comments in the functions wherever applicable.
- In PgTable.jsx please correct the spelling of 'CLOUMNS'. Not in your PR but we should fix it.
web/pgadmin/dashboard/__init__.py
Outdated
Comment on lines
565
to
568
| @blueprint.route('/logs/<frm>/<md>/<int:sid>/<int:page>', | ||
| endpoint='get_logs_by_server_id') | ||
| @blueprint.route('/logs/<frm>/<md>/<int:sid>/<int:page>/', | ||
| endpoint='get_logs_by_server_id') |
There was a problem hiding this comment.
Do we need both of these routes?
web/pgadmin/dashboard/__init__.py
Outdated
| endpoint='get_logs_by_server_id') | ||
| @login_required | ||
| @check_precondition | ||
| def logs(frm=None, md=None, sid=None, page=0): |
There was a problem hiding this comment.
Please give meaningful names to the variables "frm", and "md".
web/pgadmin/dashboard/__init__.py
Outdated
Comment on lines
598
to
599
| st = 0 | ||
| ed = ON_DEMAND_LOG_COUNT |
There was a problem hiding this comment.
Please give meaningful names to the variables "st", and "ed".
web/pgadmin/dashboard/__init__.py
Outdated
Comment on lines
644
to
645
| col1 = [] | ||
| col2 = [] |
There was a problem hiding this comment.
Remove unused variables.
web/pgadmin/dashboard/__init__.py
Outdated
| _pattern = re.compile(LOG_STATEMENTS) | ||
| for f in final_res: | ||
| tmp = re.search(LOG_STATEMENTS, f) | ||
| fsp = _pattern.split(f) |
There was a problem hiding this comment.
Remove unused variable.
| @@ -0,0 +1,6 @@ | |||
| /*pga4dash*/ | |||
| @@ -0,0 +1,8 @@ | |||
| /*pga4dash*/ | |||
| @@ -0,0 +1,8 @@ | |||
| /*pga4dash*/ | |||
| {% if log_format != '' %} | |||
| SELECT pg_read_file(pg_current_logfile('{{log_format}}'), {{ st }}, {{ ed }}); | |||
There was a problem hiding this comment.
Please give meaningful names to the variables "st", and "ed".
web/pgadmin/dashboard/__init__.py
Outdated
| LOG_STATEMENTS = 'DEBUG:|STATEMENT:|LOG:|WARNING:|NOTICE:|INFO:' \ | ||
| '|ERROR:|FATAL:|PANIC:' | ||
| if not sid: | ||
| return internal_server_error(errormsg='Server ID not specified.') |
There was a problem hiding this comment.
'Server ID not specified.' should be wrapped into the gettext. Maybe create a constant and replace it on 7 other places.
web/pgadmin/misc/__init__.py
Outdated
| @@ -228,6 +228,7 @@ def validate_binary_path(): | |||
| running the utilities with their versions. | |||
| """ | |||
| data = None | |||
| return precondition_required(gettext('Invalid binary path.')) | |||
There was a problem hiding this comment.
Remove this code as it always returns 'Invalid binary path.'
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.
No description provided.