Allow usage of regex in `LOG_FILTER` setting #3108

sonologic · 2023-03-13T09:39:08Z

Resolves: #2893

Ensured tests pass and (if applicable) updated functional test output
Conformed to code style guidelines by running appropriate linting tools
Added tests for changed code
Updated documentation for changed code

sonologic · 2023-03-13T09:40:24Z

Note that linting is broken:

(venv) gmc@sunra:~/src/3rdparty/pelican/pelican$ invoke lint
usage: flake8 [options] file file ...
flake8: error: unrecognized arguments: --diff
(venv) gmc@sunra:~/src/3rdparty/pelican/pelican$

I used PyCharm though, and that will warn me when I try to violate PEP8 and a whole bunch of other rules.

justinmayer · 2023-03-27T08:27:06Z

Note that linting is broken

I don't get the same result. I believe that is because you are using a newer version of Flake8, whereas for now we have pinned that linter dependency to version 3.9.*. Please try python -m pip install "flake8~=3.9.0" and see if that rectifies that particular problem.

Regarding the tests, they are currently failing due to changes in Pelican's Python package dependency versions, which has nothing to do with the enhancements in this pull request. I submitted PR #3112 to address this. Once that PR has been merged, this PR can be rebased in order to ensure the changes here pass all tests.

sonologic · 2023-03-27T08:32:38Z

Note that linting is broken

I don't get the same result. I believe that is because you are using a newer version of Flake8, whereas for now we have pinned that linter dependency to version 3.9.*. Please try python -m pip install "flake8~=3.9.0" and see if that rectifies that particular problem.

Yep, downgrading fixes it. Note that I used the requirements specifications in the requirements subdir. Specifically, requirements/style.pip specifies an unqualified flake8 install so it got the latest indeed.

Regarding the tests, they are currently failing due to changes in Pelican's Python package dependency versions, which has nothing to do with the enhancements in this pull request. I submitted PR #3112 to address this. Once that PR has been merged, this PR can be rebased in order to ensure the changes here pass all tests.

Cool, thanks in advance!

justinmayer · 2023-03-27T08:37:28Z

Note that I used the requirements specifications in the requirements subdir. Specifically, requirements/style.pip specifies an unqualified flake8 install so it got the latest indeed.

Thanks for pointing that out. Fixed in master via 06c9e0f. 👍

avaris · 2023-04-20T01:09:35Z

pelican/settings.py

+ for item in log_filter:
+ if len(item) == 2: # old-style string or template
+ LimitFilter.ignore.update({item})
+ elif len(item) == 3: # new-style string/template or regexp
+ if item[0] == "string":
+ LimitFilter.ignore.update({(item[1:])})
+ elif item[0] == "regex":
+ regex = re.compile(item[2])
+ LimitFilter.ignore_regexp.update({(item[1], regex)})
+ else:
+ raise ValueError(f"Invalid LOG_FILTER type '{item[0]}'")
+ else:
+ raise ValueError(f"Invalid item '{str(item)}' in LOG_FILTER")


I think it makes more sense to move this logic to LimitFilter instead, say a @classmethod named add_ignore_rule.

Also, LimitFilter.ignore.update({item}) is a long way of writing LimitFilter.ignore.add(item) :).

avaris · 2023-04-20T01:16:52Z

docs/settings.rst

+ (logging.WARN, 'TAG_SAVE_AS is set to False'),
+ ('string', logging.WARN, 'Empty theme folder. Using `basic` theme.'),


If I understand correctly, 2 item one and the one starting with "string" are essentially same thing. And I'm guessing, "string" is added to make it consistent(?) with "regex". This duplication is a bit redundant. I'd much rather have the "2 item" version deprecated but internally still supported by auto-changing it to "3 item string" version with a warning issued.

justinmayer · 2023-07-26T06:49:37Z

@sonologic: I updated your PR branch on latest master, and as expected, the linter problems have been resolved. 🌟

Would you be so kind as to address the feedback from @avaris above so we can get your contribution merged?

sonologic · 2023-10-30T07:18:50Z

@sonologic: I updated your PR branch on latest master, and as expected, the linter problems have been resolved. 🌟

Would you be so kind as to address the feedback from @avaris above so we can get your contribution merged?

I'm sorry, I've really neglected this PR, life kinda got in the way. Let me carve out some time to continue work on this so we can get it merged. I'll address the comments indeed.

Apologies for the delay.

justinmayer · 2023-10-30T08:13:52Z

That would be great, Koen. When do you think you might be able to find time to address that feedback? We are planning to release Pelican 4.9 tomorrow or Wednesday, and it would be wonderful to also include your contribution. ✨

Fix getpelican#2893

sonologic · 2023-10-30T18:15:19Z

That would be great, Koen. When do you think you might be able to find time to address that feedback? We are planning to release Pelican 4.9 tomorrow or Wednesday, and it would be wonderful to also include your contribution. ✨

No pressure then :)

I think I have addressed all the comments just now. Let's see what CI thinks of it, but imho it's good to go now.

justinmayer · 2023-10-30T18:40:48Z

Thanks for jumping on this so quickly! Looks like all CI checks are green.

I believe @avaris will take one last look to make sure everything looks okay, as soon as he has a moment.

avaris · 2023-10-30T19:01:11Z

I am looking at #3038 at the same time and that refactors how logging is done in a major way. That one and this are conflicting with each other currently. I'd prefer to merge that first and handle this one later adopting the code.

So, I wish I had seen the latest comments earlier. That might've saved you some (maybe) unnecessary work at this time, sorry @sonologic :(.

sonologic · 2023-10-30T19:07:10Z

I am looking at #3038 at the same time and that refactors how logging is done in a major way. That one and this are conflicting with each other currently. I'd prefer to merge that first and handle this one later adopting the code.

So, I wish I had seen the latest comments earlier. That might've saved you some (maybe) unnecessary work at this time, sorry @sonologic :(.

Well, it was only an hour, but I don't think I can make more time available on such short notice, so this feature will have to wait for a future release then.

sonologic · 2023-10-30T19:09:29Z

I am looking at #3038 at the same time and that refactors how logging is done in a major way. That one and this are conflicting with each other currently. I'd prefer to merge that first and handle this one later adopting the code.
So, I wish I had seen the latest comments earlier. That might've saved you some (maybe) unnecessary work at this time, sorry @sonologic :(.

Well, it was only an hour, but I don't think I can make more time available on such short notice, so this feature will have to wait for a future release then.

Did you already merge it? The conflicts don't seem all that extensive, I might fit in a few minutes to look at them tomorrow, depending on how crazy things get at the day job.

sonologic · 2023-10-30T19:13:34Z

I am looking at #3038 at the same time and that refactors how logging is done in a major way. That one and this are conflicting with each other currently. I'd prefer to merge that first and handle this one later adopting the code.
So, I wish I had seen the latest comments earlier. That might've saved you some (maybe) unnecessary work at this time, sorry @sonologic :(.

Well, it was only an hour, but I don't think I can make more time available on such short notice, so this feature will have to wait for a future release then.

Did you already merge it? The conflicts don't seem all that extensive, I might fit in a few minutes to look at them tomorrow, depending on how crazy things get at the day job.

Just looked at the MR. Seems indeed like this PR can be thrown in the bin, and I need to redo it for the new thing. That for sure is not going to happen this week. So I guess we can close this PR without merging, and I'll see when I can redo the work. Probably will be a few months again :)

justinmayer · 2023-10-30T19:13:58Z

These conflicts are the result of merging #3231, which applies ruff format . to the entire code base. We held off as long as we could, knowing it would cause some merge conflicts for the few PRs in the queue.

avaris · 2023-10-30T19:19:54Z

Well, it was only an hour, but I don't think I can make more time available on such short notice, so this feature will have to wait for a future release then.

No worries, I can make changes from here. If possible I can handle fixing conflicts.

MinchinWeb · 2023-11-26T04:11:09Z

Just an encouragement... I would use this feature if it was in Pelican!

My particular use-case is I'm building a Reader using the Markdown-IT library, but if you set the log level to debug, it logs every token processing, which quickly amounts to hundreds of lines! This would make it simple to all go away...

sonologic · 2023-12-06T07:35:07Z

Well, it was only an hour, but I don't think I can make more time available on such short notice, so this feature will have to wait for a future release then.

No worries, I can make changes from here. If possible I can handle fixing conflicts.

Curious how this went. Did the release go out? Are the refactorings in? Should I give this one another go?

avaris · 2023-12-06T17:32:33Z

Curious how this went. Did the release go out? Are the refactorings in? Should I give this one another go?

4.9 is released, but without #3038. It wasn't ready. I'm still waiting some activity on that. So... I can't really give a timeline. I can ping here when things move forward and this PR is ready for more work, if you would like to resolve that as well. But my offer stands, I'd be happy to lend a hand and handle that part if it won't be convenient for you.

sonologic marked this pull request as draft March 13, 2023 09:43

sonologic force-pushed the pr/log_filter_regexp branch from 9fe55f2 to 38701d0 Compare March 13, 2023 09:45

sonologic marked this pull request as ready for review March 13, 2023 09:45

justinmayer requested a review from avaris March 27, 2023 08:27

avaris requested changes Apr 20, 2023

View reviewed changes

justinmayer added the needs more work label Jul 26, 2023

avaris mentioned this pull request Oct 29, 2023

Ensure proper log line/filename in warning/errors #3038

Open

4 tasks

sonologic force-pushed the pr/log_filter_regexp branch from ae97fb9 to 56a9ff1 Compare October 30, 2023 17:19

Allow usage of regex in LOG_FILTER setting

284ea41

Fix getpelican#2893

justinmayer requested a review from avaris October 30, 2023 18:10

sonologic force-pushed the pr/log_filter_regexp branch from 56a9ff1 to 284ea41 Compare October 30, 2023 18:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow usage of regex in `LOG_FILTER` setting #3108

Allow usage of regex in `LOG_FILTER` setting #3108

sonologic commented Mar 13, 2023

sonologic commented Mar 13, 2023

justinmayer commented Mar 27, 2023

sonologic commented Mar 27, 2023

justinmayer commented Mar 27, 2023

avaris Apr 20, 2023

avaris Apr 20, 2023 •

edited

justinmayer commented Jul 26, 2023

sonologic commented Oct 30, 2023

justinmayer commented Oct 30, 2023

sonologic commented Oct 30, 2023

justinmayer commented Oct 30, 2023

avaris commented Oct 30, 2023

sonologic commented Oct 30, 2023

sonologic commented Oct 30, 2023

sonologic commented Oct 30, 2023

justinmayer commented Oct 30, 2023

avaris commented Oct 30, 2023

MinchinWeb commented Nov 26, 2023

sonologic commented Dec 6, 2023

avaris commented Dec 6, 2023

		(logging.WARN, 'TAG_SAVE_AS is set to False'),
		('string', logging.WARN, 'Empty theme folder. Using `basic` theme.'),

Allow usage of regex in LOG_FILTER setting #3108

Are you sure you want to change the base?

Allow usage of regex in LOG_FILTER setting #3108

Conversation

sonologic commented Mar 13, 2023

sonologic commented Mar 13, 2023

justinmayer commented Mar 27, 2023

sonologic commented Mar 27, 2023

justinmayer commented Mar 27, 2023

avaris Apr 20, 2023

Choose a reason for hiding this comment

avaris Apr 20, 2023 • edited

Choose a reason for hiding this comment

justinmayer commented Jul 26, 2023

sonologic commented Oct 30, 2023

justinmayer commented Oct 30, 2023

sonologic commented Oct 30, 2023

justinmayer commented Oct 30, 2023

avaris commented Oct 30, 2023

sonologic commented Oct 30, 2023

sonologic commented Oct 30, 2023

sonologic commented Oct 30, 2023

justinmayer commented Oct 30, 2023

avaris commented Oct 30, 2023

MinchinWeb commented Nov 26, 2023

sonologic commented Dec 6, 2023

avaris commented Dec 6, 2023

Allow usage of regex in `LOG_FILTER` setting #3108

Allow usage of regex in `LOG_FILTER` setting #3108

avaris Apr 20, 2023 •

edited