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

Issue 3342 - RFE logconv.pl should have a replacement in CLI tools #6444

Merged
merged 3 commits into from
Jan 23, 2025

Conversation

jchapma
Copy link
Contributor

@jchapma jchapma commented Dec 10, 2024

Bug description:
Perl DB is not available in RHEL 10 so the server access log
analyzer tool (logconv.pl) needs to be ported to python.

Fix description:
Initial draft of logconv.py, this is a work in progress. This
commit message will be updated following code review/rework.

Fixes: #3342

Reviewed by:

@jchapma jchapma added the work in progress Work in Progress - can be reviewed, but not ready for merge. label Dec 10, 2024
Copy link
Contributor

@progier389 progier389 left a comment

Choose a reason for hiding this comment

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

As agreed in team meeting - you can merge this first version to keep trace of it
(knowing it will evolve)

self.notesA = {}
self.notesF = {}
self.notesP = {}
self.notesU = {}
Copy link
Contributor

Choose a reason for hiding this comment

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

There is now a "notes=M" for MFA

@progier389
Copy link
Contributor

Looks fundamentally good (at least at the design level)

[1] IMHO you should clean up the style before the first commit:

$ pip install pycodestyle
$ pycodestyle --max-line-length=500 logconv.py

reports lots of issues (mostly about spaces) (BTW: I ignored the long line because IMHO keeping the long error message in a single line is painful but more readable and less error prone than splitting them)
[2] You should also create a specific man page and modify the spec file so that logconv or logconv.py get delivered in the r (that said, you may decide to delay these two changes in a future commit)
[3] Command should start with #!/usr/bin/python3 as our other python commands
[4] the global constant dicts should probably better be uppercase

Bug description:
	Perl DB is not available in RHEL 10 so the server access log
	analyzer tool (logconv.pl) needs to be ported to python.

Fix description:
	Initial draft of logconv.py, this is a work in progress. This
	commit message will be updated following code review/rework.

Fixes: 389ds#3342

Reviewed by:
logging, recommentations, other tidy up
Makefile.am, 389-ds-base.spec.in, more tidy up, removed connection group layer,
even more tidy up, processed time hours bug, file order bug
@jchapma jchapma merged commit c5e1fb1 into 389ds:main Jan 23, 2025
9 checks passed
jchapma added a commit that referenced this pull request Jan 23, 2025
…6444)

Bug description:
	Perl DB is not available in RHEL 10 so the server access log
	analyzer tool (logconv.pl) needs to be ported to python.

Fix description:
	Initial draft of logconv.py, this is a work in progress. This
	commit message will be updated following code review/rework.

Fixes: #3342

Reviewed by: @progier389 @mreynolds389  (Thank you)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
work in progress Work in Progress - can be reviewed, but not ready for merge.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

logconv.pl should have a replacement in CLI tools
3 participants