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

Searching other log files should be a Class Function #12

Open
PFython opened this issue Dec 14, 2022 · 2 comments
Open

Searching other log files should be a Class Function #12

PFython opened this issue Dec 14, 2022 · 2 comments
Labels
enhancement New feature or request

Comments

@PFython
Copy link
Owner

PFython commented Dec 14, 2022

FYI I've already changed the argument name from "logname" to "path" which is shorter and better reflects the pathlib.Path syntax elsewhere in log2d. Hope that's ok Mike?

Anyhow... this enhancement is to change the way you access other log files. Your original solution requires people to create an instance of the Log class first and give it a name. I think a better and conceptually better approach would be for people to type:

Log.find(path="another_log_file.log")

or in other words make it a Class method. This might be tricky to implement because we want a .find method for individual instances too - we might have to rename it e.g. Log.find_in("another_log_file.log") and be clever about reusing the core search logic between the class method and the instance method.

If you're not familiar with class versus instance methods I appreciate this isn't going to make much sense!

Here's the test I added to test_log2d.py:

def test_find_path():
    create_dummy_log()
    result = Log.find(path="mylog.log", date=timestamp, deltadays=-3)
    assert len(result) == 6, f"FIND14: Anotherlog - Expected 6 lines found {len(result)}"
@PFython PFython added the enhancement New feature or request label Dec 14, 2022
@PFython PFython assigned PFython and unassigned PFython Dec 14, 2022
@MikeDP
Copy link
Contributor

MikeDP commented Dec 16, 2022

This makes sense.

I've got a decorator sorted that works with the current find but - spookily (?) - find works as-is if you set a default value for self, thus:

def find(self=None, text: str="", path=None,...

This seems to work fine for

Mylog = Log('mylog', to_file=True)
result = MyLog.find()
# and
result = Log.find('mylog')
# and 
result = Log("").find('mylog')

I can find nothing about this approach, so I'm a little worried in case it breaks something in python - it has the feel of quick-and-dirty about it, so I'm inclined to use the standard decorator approach. Any comments?

@MikeDP
Copy link
Contributor

MikeDP commented Mar 28, 2023

Fixed in pull request #17

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants