Skip to content

Made Logger working#1

Open
justSam13 wants to merge 1 commit intoJh123x:mainfrom
justSam13:main
Open

Made Logger working#1
justSam13 wants to merge 1 commit intoJh123x:mainfrom
justSam13:main

Conversation

@justSam13
Copy link

The logger is working now.
Also changed some layout of the timer message.
Half implemented the storage functionality. Will complete more later.

Merge if you like it. I would love to contribute more. :)

The logger is working now.
Also changed some layout of the timer message.
Half implemented the storage functionality. Will complete more later.
Copy link
Owner

@Jh123x Jh123x left a comment

Choose a reason for hiding this comment

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

Thanks a lot for your contribution to the MR, especially the types for the __main__ file.

Comment on lines +119 to +120
logs
database No newline at end of file
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
logs
database
logs/
database/

Maybe we can add the / at the back to denote directories

Comment on lines +11 to +25
if os.path.isfile(f"./database/storage.json"):
logger.info("Loading stored events...")
with open("./database/storage.json", "r") as storageFile:
try:
self.storage = json.load(storageFile)
except Exception as e:
logger.error("Error loading stored events", {str(e)})
self.storage = {}
else:
logger.info("No saved events found.")
self.storage = {}
os.makedirs("./database", exist_ok=True)
with open("./database/storage.json", "w") as storageFile:
json.dump(self.storage, storageFile, indent=4)

Copy link
Owner

Choose a reason for hiding this comment

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

This is not an ideal solution for writing into a file.
If server is stopped unexpectedly, the events will not saved.

In this case it might be better to use something like sqlite3.

"""Stores the information for each user"""
# Load the shelve db if possible
self.storage = {}
if os.path.isfile(f"./database/storage.json"):
Copy link
Owner

Choose a reason for hiding this comment

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

  BASE_DIR = os.path.dirname(os.path.abspath(__file__))
  ...
  path = os.path.join(BASE_DIR,"database","storage.json")

Filepath can be improved using the snippet above instead of ./ as the location may vary based on the directory where the command was run.

Comment on lines +26 to +30
format = "(black){asctime}(reset) (levelcolor){levelname}(reset) (green){name}(reset) {message}"
format = format.replace("(black)", self.black + self.bold)
format = format.replace("(reset)", self.reset)
format = format.replace("(levelcolor)", log_color)
format = format.replace("(green)", self.green + self.bold)
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
format = "(black){asctime}(reset) (levelcolor){levelname}(reset) (green){name}(reset) {message}"
format = format.replace("(black)", self.black + self.bold)
format = format.replace("(reset)", self.reset)
format = format.replace("(levelcolor)", log_color)
format = format.replace("(green)", self.green + self.bold)
format = "{black}{{asctime}}{reset} {levelcolor}{{levelname}}{reset} {green}{{name}}{reset} {{message}}".format(
black= self.black + self.bold,
reset = self.reset,
levelcolor=log_color,
green=self.green+self.bold,
)

We can make use of {{ and }} to escape the { and } so that we can format them later.

"""
Throws ValueError when the format is incorrect
"""
chat_id = str(chat_id)
Copy link
Owner

Choose a reason for hiding this comment

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

Maybe we can create another class to deal with the file logging (opening/closing/writing to files ) and only write to the file when the destructor of the class is called.

seconds = time.seconds % 60
return TIME_FORMAT.format(days=time.days, hours=hours, minutes=minutes, seconds=seconds)

time_string = ""
Copy link
Owner

Choose a reason for hiding this comment

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

The formatting logic here can be handled by changing the TIME_FORMAT variable


# Logger Format
LOGGER_FORMAT = '%(asctime)s %(clientip)-15s %(user)-8s %(message)s'
# LOGGER_FORMAT = '%(asctime)s %(clientip)-15s %(user)-8s %(message)s'
Copy link
Owner

Choose a reason for hiding this comment

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

Maybe we can remove commented code. If we have a need to retrieve it, we can look at the previous versions of the code.

@Jh123x Jh123x added the enhancement New feature or request label May 9, 2024
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

Successfully merging this pull request may close these issues.

2 participants