[PATCH v1 00/13] Improvements, refactors, and features

[PATCH v1 00/13] Improvements, refactors, and features

[Muhammad Rizki]

Hir sir,
This is v1 of Improvements, refactors, and features. This series is to
move a function, code Improvements, add a function, and add an error
class.

In the last series, I didn't care so much with the Discord
implementation. So, I forget to add a reconnect MySQL method for the
Discord bot. In this series, I've added it and implement it in the
listener.py exactly in the handle_db_error().

In the last commit, Telegram report_err() is implemented in the
listener.py. In this series, I've been move it to the Telegram bot
custom client instead. I've added a report_err() for the Discord bot
too in the client.

I've added DaemonException() error class too to easily save a thread URL
if errors may happen, this error class is to easily reproducing errors.

Some unnecessary try/except block is already removed and add the
try/except block again in the __handle_atom_url(), we need to catch the
thread URL to easily reproducing errors if errors may happen in the
future.

I've implemented handle_db_error() for the Discord bot too.

What's new?
- Added a report_err() for the Discord bot.
- Added a DaemonException() error class for both bots.
- Added a ping() for reconnecting MySQL conn for the Discord bot.
- Added a handle_db_error() for the Discord bot mail listener.

Refactors:
- Move report_err() to the DaemonTelegram() for the Telegram bot.
- Remove try/except for both bots in their listener.py.
- Ensure the Telegram bot has been started, so start the client earlier.
- Rename the Telegram bot custom client to "DaemonTelegram".

There are 13 patches in this series:
- Patch 1 is to remove unnecessary try/except in the listener for DC bot.
- Patch 2 is to move report_err() from listener to DaemonTelegram().
- Patch 3 is to ensure the Telegram bot has been started before interact.
- Patch 4 is to added DaemonException() error class to handle error.
- Patch 5 is to implement the DaemonException() error class for the TG.
- Patch 6 is to remove unnecessary try/except in the listener for TG bot
- Patch 7 is to added report_err() for the Discord bot in the client.
- Patch 8 is to implement the report_err() in the Discord mail listener.
- Patch 9 is to added ping() method to reconnect MySQL connection for DC.
- Patch 10 is to added handle_db_error() in the listener for the DC bot.
- Patch 11 is to handle MySQL connection error in the listener for the DC
- Patch 12 is to implement the DaemonException() error class for the DC.
- Patch 13 is to rename the Telegram bot custom client to DaemonTelegram()

How to set log channel ID for the Telegram bot:
1. Make a channel for log in the Telegram.
2. Copy the channel ID, if you unable to see the ID you can use API or
   bot who has the feature, or use a 64gram desktop.
3. Paste the copied ID into the config.py in the telegram directory.

How to set log channel ID for the Discord bot:
1. Make a channel for log in your Discord server.
2. Copy the channel ID by right click and copy, if you unable to get the
   ID you can go to your account settings, then enable the
   `Developer mode` or use bot who has the feature.
3. Paste the copied ID into the config.py in the dscord directory.

How to use:
1. Execute the db.sql file in the daemon directory.
2. Setup .env file, the example is there with suffix .example, this
   file name must remove the suffix name .example.
3. Set up the config.py in each bot directory, such as dscord and
   telegram. The example is there with suffix .example & the file name
   must remove suffix name .example.
4. Run `pip3 install -r requirements.txt` in each bot directory.
5. STORAGE_DIR env value must `storage` to make it work fine.
6. Run the bot by `python3 dc.py` or `python3 tg.py`.

Please give it a test and give me more comments or suggestion, thanks!

Signed-off-by: Muhammad Rizki <kiizuha@gnuweeb.org>
---

Muhammad Rizki (13):
  refactor(discord): Remove unnecessary try/except block
  refactor(telegram): Move report_err() to the DaemonClient() class
  refactor(telegram)!: Ensure the Telegram bot has been started
  feat: add DaemonException() class
  feat(telegram): Implement the DaemonException() class
  refactor(telegram/mailer)!: Remove unnecessary try/except block
  feat(discord): add report_err() for the Discord bot
  feat(discord): Implement the report_err()
  feat(discord/database): Add ping() method
  feat(discord): Add handle_db_error in the listener.py
  feat(discord): Handle MySQL connection error in the mail listener
  feat(discord): Implement the DaemonException for handle error
  refactor(telegram)!: Rename the Telegram bot custom client

 daemon/dscord/database/core.py                |  6 +++
 daemon/dscord/gnuweeb/client.py               |  8 ++++
 daemon/dscord/mailer/listener.py              | 48 ++++++++++++++-----
 daemon/exceptions.py                          | 18 +++++++
 daemon/telegram/mailer/listener.py            | 36 +++++++-------
 daemon/telegram/packages/__init__.py          |  2 +-
 daemon/telegram/packages/client.py            | 10 +++-
 daemon/telegram/packages/decorator.py         |  2 +-
 .../packages/plugins/callbacks/del_atom.py    |  6 +--
 .../packages/plugins/callbacks/del_chat.py    |  6 +--
 .../packages/plugins/commands/manage_atom.py  | 10 ++--
 .../plugins/commands/manage_broadcast.py      | 10 ++--
 .../packages/plugins/commands/scrape.py       |  6 +--
 daemon/tg.py                                  | 11 +++--
 14 files changed, 120 insertions(+), 59 deletions(-)
 create mode 100644 daemon/exceptions.py


base-commit: d72337c346686b7faa48ee654c967dba1dfebea0
--
Muhammad Rizki

[PATCH v1 01/13] refactor(discord): Remove unnecessary try/except block

[Muhammad Rizki]

We don't need this anymore since it's already has a try/except earlier.

Signed-off-by: Muhammad Rizki <kiizuha@gnuweeb.org>
---
 daemon/dscord/mailer/listener.py | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/daemon/dscord/mailer/listener.py b/daemon/dscord/mailer/listener.py
index 24f0a6c..a225f46 100644
--- a/daemon/dscord/mailer/listener.py
+++ b/daemon/dscord/mailer/listener.py
@@ -106,14 +106,7 @@ class Listener:
 			self.logger.debug(md)
 			return False
 
-		try:
-			text, files, is_patch = utils.create_template(mail, Platform.DISCORD)
-		except:
-			exc_str = utils.catch_err()
-			self.client.logger.warning(exc_str)
-			await self.client.send_log_file(url)
-			return
-
+		text, files, is_patch = utils.create_template(mail, Platform.DISCORD)
 		reply_to = self.get_discord_reply(mail, dc_chat_id)
 		url = str(re.sub(r"/raw$", "", url))
 
-- 
Muhammad Rizki

[PATCH v1 02/13] refactor(telegram): Move report_err() to the DaemonClient() class

[Muhammad Rizki]

Instead of adding the report_err() function in the Bot() class, move it
to the DaemonClient() class.

Signed-off-by: Muhammad Rizki <kiizuha@gnuweeb.org>
---
 daemon/telegram/mailer/listener.py | 8 +-------
 daemon/telegram/packages/client.py | 8 ++++++++
 2 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/daemon/telegram/mailer/listener.py b/daemon/telegram/mailer/listener.py
index 044de6a..8425dd4 100644
--- a/daemon/telegram/mailer/listener.py
+++ b/daemon/telegram/mailer/listener.py
@@ -61,12 +61,6 @@ class Bot():
 		self.db.ping(reconnect=True, attempts=reconnect_attempts,
 			     delay=delay_in_secs)
 
-	async def report_err(caption):
-		if not caption:
-			caption = "No lore URL"
-		exc_str = utils.catch_err()
-		self.logger.warning(exc_str)
-		await self.client.send_log_file(caption)
 
 	async def __run(self):
 		self.logger.info("Running...")
@@ -77,7 +71,7 @@ class Bot():
 		except (OperationalError, DatabaseError) as e:
 			await self.handle_db_error(e)
 		except:
-			await self.report_err(url)
+			await self.client.report_err(url)
 
 		if not self.isRunnerFixed:
 			self.isRunnerFixed = True
diff --git a/daemon/telegram/packages/client.py b/daemon/telegram/packages/client.py
index 58195b9..f543caa 100644
--- a/daemon/telegram/packages/client.py
+++ b/daemon/telegram/packages/client.py
@@ -27,6 +27,14 @@ class DaemonClient(Client):
 		self.logger = logger
 
 
+	async def report_err(self, caption):
+		if not caption:
+			caption = "No lore URL"
+		exc_str = utils.catch_err()
+		self.logger.warning(exc_str)
+		await self.send_log_file(caption)
+
+
 	@handle_flood
 	async def send_log_file(self, caption: str):
 		filename = self.logger.handlers[0].baseFilename
-- 
Muhammad Rizki

[PATCH v1 03/13] refactor(telegram)!: Ensure the Telegram bot has been started

[Muhammad Rizki]

We need to start the Telegram bot first before the listener is started.
So, if error may happen, when the listener sending a report message to
the channel, the Telegram bot is already been started.

Signed-off-by: Muhammad Rizki <kiizuha@gnuweeb.org>
---
 daemon/telegram/mailer/listener.py | 1 +
 daemon/tg.py                       | 7 +++++--
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/daemon/telegram/mailer/listener.py b/daemon/telegram/mailer/listener.py
index 8425dd4..eed3953 100644
--- a/daemon/telegram/mailer/listener.py
+++ b/daemon/telegram/mailer/listener.py
@@ -37,6 +37,7 @@ class Bot():
 		# Execute __run() once to avoid high latency at
 		# initilization.
 		#
+		self.sched.start()
 		self.runner = self.sched.add_job(
 			func=self.__run,
 			misfire_grace_time=None,
diff --git a/daemon/tg.py b/daemon/tg.py
index 73ff2b9..9d50e44 100644
--- a/daemon/tg.py
+++ b/daemon/tg.py
@@ -7,6 +7,7 @@
 from apscheduler.schedulers.asyncio import AsyncIOScheduler
 from dotenv import load_dotenv
 from mysql import connector
+from pyrogram import idle
 from atom import Scraper
 from telegram.packages import DaemonClient
 from telegram.mailer import BotMutexes
@@ -58,9 +59,11 @@ def main():
 		scraper=Scraper(),
 		mutexes=BotMutexes()
 	)
-	sched.start()
+
+	client.start()
 	bot.run()
-	client.run()
+	idle()
+	client.stop()
 
 
 if __name__ == '__main__':
-- 
Muhammad Rizki

[PATCH v1 04/13] feat: add DaemonException() class

[Muhammad Rizki]

Added DaemonException() class in the daemon/exceptions.py to be able to
throw custom error, the use case is to easily catch the email thread URL

Signed-off-by: Muhammad Rizki <kiizuha@gnuweeb.org>
---
 daemon/exceptions.py | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)
 create mode 100644 daemon/exceptions.py

diff --git a/daemon/exceptions.py b/daemon/exceptions.py
new file mode 100644
index 0000000..34f6823
--- /dev/null
+++ b/daemon/exceptions.py
@@ -0,0 +1,18 @@
+from typing import Optional
+
+class DaemonException(Exception):
+	thread_url: Optional[str] = None
+	atom_url: Optional[str] = None
+	original_exception: Optional[str] = None
+
+	def __str__(self) -> str:
+		return self.original_exception
+
+	def set_atom_url(self, url: str):
+		self.atom_url = url
+
+	def set_thread_url(self, url: str):
+		self.thread_url = url
+
+	def set_message(self, msg: str):
+		self.original_exception = msg
-- 
Muhammad Rizki

[PATCH v1 05/13] feat(telegram): Implement the DaemonException() class

[Muhammad Rizki]

This exception class is to handle error that are easy to catch the email
thread URL, this class has been customize and are inherited from the
built-in Exception() class.

Signed-off-by: Muhammad Rizki <kiizuha@gnuweeb.org>
---
 daemon/telegram/mailer/listener.py | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/daemon/telegram/mailer/listener.py b/daemon/telegram/mailer/listener.py
index eed3953..d2d9a5e 100644
--- a/daemon/telegram/mailer/listener.py
+++ b/daemon/telegram/mailer/listener.py
@@ -8,6 +8,7 @@ from pyrogram.types import Message
 from mysql.connector.errors import OperationalError, DatabaseError
 from apscheduler.schedulers.asyncio import AsyncIOScheduler
 from telegram.packages import DaemonClient
+from exceptions import DaemonException
 from atom import Scraper
 from atom import utils
 from enums import Platform
@@ -71,8 +72,9 @@ class Bot():
 				await self.__handle_atom_url(url)
 		except (OperationalError, DatabaseError) as e:
 			await self.handle_db_error(e)
-		except:
-			await self.client.report_err(url)
+		except DaemonException as e:
+			e.set_atom_url(url)
+			await self.client.report_err(e.thread_url)
 
 		if not self.isRunnerFixed:
 			self.isRunnerFixed = True
@@ -88,8 +90,14 @@ class Bot():
 	async def __handle_atom_url(self, url):
 		urls = await self.scraper.get_new_threads_urls(url)
 		for url in urls:
-			mail = await self.scraper.get_email_from_url(url)
-			await self.__handle_mail(url, mail)
+			try:
+				mail = await self.scraper.get_email_from_url(url)
+				await self.__handle_mail(url, mail)
+			except:
+				e = DaemonException()
+				e.set_thread_url(url)
+				e.set_message(utils.catch_err())
+				raise e
 
 
 	async def __handle_mail(self, url, mail):
-- 
Muhammad Rizki

[PATCH v1 06/13] refactor(telegram/mailer)!: Remove unnecessary try/except block

[Muhammad Rizki]

The error handling is already been moved to the __handle_atom_url()
so it's not necessary to use try/except block again.

Signed-off-by: Muhammad Rizki <kiizuha@gnuweeb.org>
---
 daemon/telegram/mailer/listener.py | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/daemon/telegram/mailer/listener.py b/daemon/telegram/mailer/listener.py
index d2d9a5e..e906b1f 100644
--- a/daemon/telegram/mailer/listener.py
+++ b/daemon/telegram/mailer/listener.py
@@ -127,14 +127,7 @@ class Bot():
 			self.logger.debug(md)
 			return False
 
-		try:
-			text, files, is_patch = utils.create_template(mail, Platform.TELEGRAM)
-		except:
-			exc_str = utils.catch_err()
-			self.logger.warning(exc_str)
-			await self.client.send_log_file(url)
-			return
-
+		text, files, is_patch = utils.create_template(mail, Platform.TELEGRAM)
 		reply_to = self.get_reply(mail, tg_chat_id)
 		url = str(re.sub(r"/raw$", "", url))
 
-- 
Muhammad Rizki

[PATCH v1 07/13] feat(discord): add report_err() for the Discord bot

[Muhammad Rizki]

Added report_err() in the Discord bot GWClient() class for reporting
error to the Discord channel.

Signed-off-by: Muhammad Rizki <kiizuha@gnuweeb.org>
---
 daemon/dscord/gnuweeb/client.py | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/daemon/dscord/gnuweeb/client.py b/daemon/dscord/gnuweeb/client.py
index 406c606..563d073 100644
--- a/daemon/dscord/gnuweeb/client.py
+++ b/daemon/dscord/gnuweeb/client.py
@@ -40,6 +40,14 @@ class GWClient(commands.Bot):
 		)
 
 
+	async def report_err(self, caption):
+		if not caption:
+			caption = "No lore URL"
+		exc_str = utils.catch_err()
+		self.logger.warning(exc_str)
+		await self.send_log_file(caption)
+
+
 	@filters.wait_on_limit
 	async def send_log_file(self, caption: str):
 		filename = self.logger.handlers[0].baseFilename
-- 
Muhammad Rizki

[PATCH v1 08/13] feat(discord): Implement the report_err()

[Muhammad Rizki]

After report_err() is created within the GWClient() class, implement it
in the dscord/mailer/listener.py for reporting error log to the Discord
channel.

Signed-off-by: Muhammad Rizki <kiizuha@gnuweeb.org>
---
 daemon/dscord/mailer/listener.py | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/daemon/dscord/mailer/listener.py b/daemon/dscord/mailer/listener.py
index a225f46..b45c9d0 100644
--- a/daemon/dscord/mailer/listener.py
+++ b/daemon/dscord/mailer/listener.py
@@ -51,8 +51,13 @@ class Listener:
 
 	async def __run(self):
 		self.logger.info("Running...")
-		for url in self.db.get_atom_urls():
-			await self.__handle_atom_url(url)
+		url = None
+
+		try:
+			for url in self.db.get_atom_urls():
+				await self.__handle_atom_url(url)
+		except:
+			await self.client.report_err(url)
 
 		if not self.isRunnerFixed:
 			self.isRunnerFixed = True
@@ -72,9 +77,8 @@ class Listener:
 				mail = await self.scraper.get_email_from_url(url)
 				await self.__handle_mail(url, mail)
 			except:
-				exc_str = utils.catch_err()
-				self.client.logger.warning(exc_str)
-				await self.client.send_log_file(url)
+				# this statement will be changed in the next commit
+				await self.client.report_err(url)
 
 
 	async def __handle_mail(self, url, mail):
-- 
Muhammad Rizki

[PATCH v1 09/13] feat(discord/database): Add ping() method

[Muhammad Rizki]

Added ping() in the Discord database core.py for reconnecting the MySQL
database. This function is to reconnect the MySQL connection when the
service has been restarted.

Signed-off-by: Muhammad Rizki <kiizuha@gnuweeb.org>
---
 daemon/dscord/database/core.py | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/daemon/dscord/database/core.py b/daemon/dscord/database/core.py
index c34d7a8..c8826bb 100644
--- a/daemon/dscord/database/core.py
+++ b/daemon/dscord/database/core.py
@@ -15,6 +15,12 @@ class DB(DBMethods):
 		self.cur = self.conn.cursor(buffered=True)
 
 
+	def ping(self, reconnect=True, attempts=3, delay=3):
+		self.conn.ping(reconnect=reconnect, attempts=attempts,
+				delay=delay)
+		self.cur = self.conn.cursor(buffered=True)
+
+
 	def __del__(self):
 		self.cur.close()
 		self.conn.close()
-- 
Muhammad Rizki

[PATCH v1 10/13] feat(discord): Add handle_db_error in the listener.py

[Muhammad Rizki]

After added a ping() method in the database, we should handle the
database error such as connection error. This function is to handle the
connection error while the MySQL service has been restarted.

Signed-off-by: Muhammad Rizki <kiizuha@gnuweeb.org>
---
 daemon/dscord/mailer/listener.py | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/daemon/dscord/mailer/listener.py b/daemon/dscord/mailer/listener.py
index b45c9d0..ec58f6a 100644
--- a/daemon/dscord/mailer/listener.py
+++ b/daemon/dscord/mailer/listener.py
@@ -49,6 +49,24 @@ class Listener:
 		self.runner = self.sched.add_job(func=self.__run)
 
 
+	async def handle_db_error(self, e):
+		#
+		# TODO(ammarfaizi2):
+		# Ideally, we also want to log and report this situation.
+		#
+		self.logger.error(f"Database error: {str(e)}")
+		self.logger.info("Reconnecting to the database...")
+
+		#
+		# Don't do this too often to avoid reconnect burst.
+		#
+		delay_in_secs = 3
+		reconnect_attempts = 3
+
+		self.db.ping(reconnect=True, attempts=reconnect_attempts,
+			     delay=delay_in_secs)
+
+
 	async def __run(self):
 		self.logger.info("Running...")
 		url = None
-- 
Muhammad Rizki

[PATCH v1 11/13] feat(discord): Handle MySQL connection error in the mail listener

[Muhammad Rizki]

This error handling is to reconnect the MySQL connection when sometimes
MySQL service having a problem or has been restarted while the bot is
still running.

Signed-off-by: Muhammad Rizki <kiizuha@gnuweeb.org>
---
 daemon/dscord/mailer/listener.py | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/daemon/dscord/mailer/listener.py b/daemon/dscord/mailer/listener.py
index ec58f6a..2bde0f0 100644
--- a/daemon/dscord/mailer/listener.py
+++ b/daemon/dscord/mailer/listener.py
@@ -6,6 +6,7 @@
 
 import asyncio
 import re
+from mysql.connector.errors import OperationalError, DatabaseError
 from apscheduler.schedulers.asyncio import AsyncIOScheduler
 from discord import File
 from discord import Message
@@ -74,6 +75,8 @@ class Listener:
 		try:
 			for url in self.db.get_atom_urls():
 				await self.__handle_atom_url(url)
+		except (OperationalError, DatabaseError) as e:
+			await self.handle_db_error(e)
 		except:
 			await self.client.report_err(url)
 
-- 
Muhammad Rizki

[PATCH v1 12/13] feat(discord): Implement the DaemonException for handle error

[Muhammad Rizki]

This custom error class is used to catch errors and save information
such as the email thread URL, atom URL, and exception message. The
purpose of saving this information is to report the error with the
thread URL for easily debugging.

Signed-off-by: Muhammad Rizki <kiizuha@gnuweeb.org>
---
 daemon/dscord/mailer/listener.py | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/daemon/dscord/mailer/listener.py b/daemon/dscord/mailer/listener.py
index 2bde0f0..e55a4fc 100644
--- a/daemon/dscord/mailer/listener.py
+++ b/daemon/dscord/mailer/listener.py
@@ -15,6 +15,7 @@ from dscord.gnuweeb import GWClient
 from atom.scraper import Scraper
 from atom import utils
 from enums import Platform
+from exceptions import DaemonException
 
 
 class Mutexes:
@@ -77,8 +78,9 @@ class Listener:
 				await self.__handle_atom_url(url)
 		except (OperationalError, DatabaseError) as e:
 			await self.handle_db_error(e)
-		except:
-			await self.client.report_err(url)
+		except DaemonException as e:
+			e.set_atom_url(url)
+			await self.client.report_err(e.thread_url)
 
 		if not self.isRunnerFixed:
 			self.isRunnerFixed = True
@@ -98,8 +100,10 @@ class Listener:
 				mail = await self.scraper.get_email_from_url(url)
 				await self.__handle_mail(url, mail)
 			except:
-				# this statement will be changed in the next commit
-				await self.client.report_err(url)
+				e = DaemonException()
+				e.set_thread_url(url)
+				e.set_message(utils.catch_err())
+				raise e
 
 
 	async def __handle_mail(self, url, mail):
-- 
Muhammad Rizki

[PATCH v1 13/13] refactor(telegram)!: Rename the Telegram bot custom client

[Muhammad Rizki]

Renamed all Telegram bot custom client that has been used in other files

Signed-off-by: Muhammad Rizki <kiizuha@gnuweeb.org>
---
 daemon/telegram/mailer/listener.py                     |  4 ++--
 daemon/telegram/packages/__init__.py                   |  2 +-
 daemon/telegram/packages/client.py                     |  2 +-
 daemon/telegram/packages/decorator.py                  |  2 +-
 daemon/telegram/packages/plugins/callbacks/del_atom.py |  6 +++---
 daemon/telegram/packages/plugins/callbacks/del_chat.py |  6 +++---
 .../telegram/packages/plugins/commands/manage_atom.py  | 10 +++++-----
 .../packages/plugins/commands/manage_broadcast.py      | 10 +++++-----
 daemon/telegram/packages/plugins/commands/scrape.py    |  6 +++---
 daemon/tg.py                                           |  4 ++--
 10 files changed, 26 insertions(+), 26 deletions(-)

diff --git a/daemon/telegram/mailer/listener.py b/daemon/telegram/mailer/listener.py
index e906b1f..4e1e0da 100644
--- a/daemon/telegram/mailer/listener.py
+++ b/daemon/telegram/mailer/listener.py
@@ -7,7 +7,7 @@
 from pyrogram.types import Message
 from mysql.connector.errors import OperationalError, DatabaseError
 from apscheduler.schedulers.asyncio import AsyncIOScheduler
-from telegram.packages import DaemonClient
+from telegram.packages import DaemonTelegram
 from exceptions import DaemonException
 from atom import Scraper
 from atom import utils
@@ -22,7 +22,7 @@ class BotMutexes():
 
 
 class Bot():
-	def __init__(self, client: DaemonClient, sched: AsyncIOScheduler,
+	def __init__(self, client: DaemonTelegram, sched: AsyncIOScheduler,
 			scraper: Scraper, mutexes: BotMutexes):
 		self.client = client
 		self.sched = sched
diff --git a/daemon/telegram/packages/__init__.py b/daemon/telegram/packages/__init__.py
index efef9ae..15a270f 100644
--- a/daemon/telegram/packages/__init__.py
+++ b/daemon/telegram/packages/__init__.py
@@ -1 +1 @@
-from .client import DaemonClient
+from .client import DaemonTelegram
diff --git a/daemon/telegram/packages/client.py b/daemon/telegram/packages/client.py
index f543caa..1c5f3b7 100644
--- a/daemon/telegram/packages/client.py
+++ b/daemon/telegram/packages/client.py
@@ -16,7 +16,7 @@ from telegram.database import DB
 from .decorator import handle_flood
 
 
-class DaemonClient(Client):
+class DaemonTelegram(Client):
 	def __init__(self, name: str, api_id: int,
 		api_hash: str, conn, logger: BotLogger,
 		**kwargs
diff --git a/daemon/telegram/packages/decorator.py b/daemon/telegram/packages/decorator.py
index 153fa95..efcfc38 100644
--- a/daemon/telegram/packages/decorator.py
+++ b/daemon/telegram/packages/decorator.py
@@ -24,7 +24,7 @@ def handle_flood(func: Callable[[T], T]) -> Callable[[T], T]:
 			try:
 				return await func(*args)
 			except FloodWait as e:
-				# Calling logger attr from the DaemonClient() class
+				# Calling logger attr from the DaemonTelegram() class
 				logger = args[0].logger
 
 				_flood_exceptions(e, logger)
diff --git a/daemon/telegram/packages/plugins/callbacks/del_atom.py b/daemon/telegram/packages/plugins/callbacks/del_atom.py
index b750e1c..84524c2 100644
--- a/daemon/telegram/packages/plugins/callbacks/del_atom.py
+++ b/daemon/telegram/packages/plugins/callbacks/del_atom.py
@@ -3,14 +3,14 @@
 # Copyright (C) 2022  Muhammad Rizki <riskimuhammmad1@gmail.com>
 #
 
-from telegram.packages import DaemonClient
+from telegram.packages import DaemonTelegram
 from atom import utils
 from pyrogram.types import CallbackQuery
 from telegram import config
 
 
-@DaemonClient.on_callback_query(config.admin_only, group=1)
-async def on_del_atom(c: DaemonClient, cb: CallbackQuery):
+@DaemonTelegram.on_callback_query(config.admin_only, group=1)
+async def on_del_atom(c: DaemonTelegram, cb: CallbackQuery):
 	if not "del_atom" in cb.data:
 		return
 
diff --git a/daemon/telegram/packages/plugins/callbacks/del_chat.py b/daemon/telegram/packages/plugins/callbacks/del_chat.py
index 90b557e..c7135e0 100644
--- a/daemon/telegram/packages/plugins/callbacks/del_chat.py
+++ b/daemon/telegram/packages/plugins/callbacks/del_chat.py
@@ -3,14 +3,14 @@
 # Copyright (C) 2022  Muhammad Rizki <riskimuhammmad1@gmail.com>
 #
 
-from telegram.packages import DaemonClient
+from telegram.packages import DaemonTelegram
 from atom import utils
 from pyrogram.types import CallbackQuery
 from telegram import config
 
 
-@DaemonClient.on_callback_query(config.admin_only, group=2)
-async def on_del_chat(c: DaemonClient, cb: CallbackQuery):
+@DaemonTelegram.on_callback_query(config.admin_only, group=2)
+async def on_del_chat(c: DaemonTelegram, cb: CallbackQuery):
 	if not "del_chat" in cb.data:
 		return
 
diff --git a/daemon/telegram/packages/plugins/commands/manage_atom.py b/daemon/telegram/packages/plugins/commands/manage_atom.py
index 99df7f7..f686adf 100644
--- a/daemon/telegram/packages/plugins/commands/manage_atom.py
+++ b/daemon/telegram/packages/plugins/commands/manage_atom.py
@@ -5,16 +5,16 @@
 
 from pyrogram.types import Message
 from pyrogram import filters
-from telegram.packages import DaemonClient
+from telegram.packages import DaemonTelegram
 from atom import utils
 from telegram import config
 
 
-@DaemonClient.on_message(
+@DaemonTelegram.on_message(
 	filters.command("add_atom") &
 	config.admin_only
 )
-async def add_atom_url(c: DaemonClient, m: Message):
+async def add_atom_url(c: DaemonTelegram, m: Message):
 	if len(m.command) <= 1:
 		tutor = "Please specify the URL\n"
 		tutor += "Example: `/add_atom https://lore.kernel.org/linux-sgx/new.atom`"
@@ -32,11 +32,11 @@ async def add_atom_url(c: DaemonClient, m: Message):
 	await m.reply(f"Success add **{text}** for listening new email")
 
 
-@DaemonClient.on_message(
+@DaemonTelegram.on_message(
 	filters.command("del_atom") &
 	config.admin_only
 )
-async def del_atom_url(c: DaemonClient, m: Message):
+async def del_atom_url(c: DaemonTelegram, m: Message):
 	atoms = c.db.get_atom_urls()
 	if len(atoms) == 0:
 		return await m.reply("Currently empty.")
diff --git a/daemon/telegram/packages/plugins/commands/manage_broadcast.py b/daemon/telegram/packages/plugins/commands/manage_broadcast.py
index 0aa70de..63f9547 100644
--- a/daemon/telegram/packages/plugins/commands/manage_broadcast.py
+++ b/daemon/telegram/packages/plugins/commands/manage_broadcast.py
@@ -5,16 +5,16 @@
 
 from pyrogram.types import Message
 from pyrogram import filters, enums
-from telegram.packages import DaemonClient
+from telegram.packages import DaemonTelegram
 from atom import utils
 from telegram import config
 
 
-@DaemonClient.on_message(
+@DaemonTelegram.on_message(
 	filters.command("add_bc") &
 	config.admin_only
 )
-async def add_broadcast(c: DaemonClient, m: Message):
+async def add_broadcast(c: DaemonTelegram, m: Message):
 	if m.chat.type == enums.ChatType.PRIVATE:
 		chat_name = m.chat.first_name
 	else:
@@ -37,11 +37,11 @@ async def add_broadcast(c: DaemonClient, m: Message):
 	await m.reply(msg)
 
 
-@DaemonClient.on_message(
+@DaemonTelegram.on_message(
 	filters.command("del_bc") &
 	config.admin_only
 )
-async def del_broadcast(c: DaemonClient, m: Message):
+async def del_broadcast(c: DaemonTelegram, m: Message):
 	if "--list" in m.text:
 		chats = c.db.get_broadcast_chats()
 		if len(chats) == 0:
diff --git a/daemon/telegram/packages/plugins/commands/scrape.py b/daemon/telegram/packages/plugins/commands/scrape.py
index f89727d..89581b6 100644
--- a/daemon/telegram/packages/plugins/commands/scrape.py
+++ b/daemon/telegram/packages/plugins/commands/scrape.py
@@ -6,7 +6,7 @@
 
 from pyrogram.types import Message
 from pyrogram import filters
-from telegram.packages import DaemonClient
+from telegram.packages import DaemonTelegram
 from atom import Scraper
 from atom import utils
 from enums import Platform
@@ -22,11 +22,11 @@ import asyncio
 #    .lore https://lore.kernel.org/path/message_id/raw
 #
 LORE_CMD_URL_PATTERN = r"^(?:\/|\.|\!)lore\s+(https?:\/\/lore\.kernel\.org\/\S+)"
-@DaemonClient.on_message(
+@DaemonTelegram.on_message(
 	filters.regex(LORE_CMD_URL_PATTERN) &
 	config.admin_only
 )
-async def scrap_email(c: DaemonClient, m: Message):
+async def scrap_email(c: DaemonTelegram, m: Message):
 	p = re.search(LORE_CMD_URL_PATTERN, m.text)
 	if not p:
 		return
diff --git a/daemon/tg.py b/daemon/tg.py
index 9d50e44..d3b5760 100644
--- a/daemon/tg.py
+++ b/daemon/tg.py
@@ -9,7 +9,7 @@ from dotenv import load_dotenv
 from mysql import connector
 from pyrogram import idle
 from atom import Scraper
-from telegram.packages import DaemonClient
+from telegram.packages import DaemonTelegram
 from telegram.mailer import BotMutexes
 from telegram.mailer import Bot
 from logger import BotLogger
@@ -28,7 +28,7 @@ def main():
 	else:
 		port = int(port)
 
-	client = DaemonClient(
+	client = DaemonTelegram(
 		"telegram/storage/EmailScraper",
 		api_id=int(os.getenv("API_ID")),
 		api_hash=os.getenv("API_HASH"),
-- 
Muhammad Rizki

Re: [PATCH v1 03/13] refactor(telegram)!: Ensure the Telegram bot has been started

[Ammar Faizi]

On 1/3/23 1:36 PM, Muhammad Rizki wrote:
> @@ -58,9 +59,11 @@ def main():
>   		scraper=Scraper(),
>   		mutexes=BotMutexes()
>   	)
> -	sched.start()
> +
> +	client.start()
>   	bot.run()
> -	client.run()
> +	idle()
> +	client.stop()

I agree to have an 'idle()' call, but it addresses a different issue
w.r.t. the patch subject. Please split the 'idle()' patch into a
separate patch.

My understanding is that the reason for the 'idle()' call is to make the
program gracefully exits when it's interrupted (e.g., SIGINT with
CTRL + C), then you have a chance to call 'client.stop()', which could
be a necessary cleanup to stop the app.

Is my understanding correct?

-- 
Ammar Faizi

Re: [PATCH v1 05/13] feat(telegram): Implement the DaemonException() class

[Ammar Faizi]

On 1/3/23 1:36 PM, Muhammad Rizki wrote:
> @@ -71,8 +72,9 @@ class Bot():
>   				await self.__handle_atom_url(url)
>   		except (OperationalError, DatabaseError) as e:
>   			await self.handle_db_error(e)
> -		except:
> -			await self.client.report_err(url)
> +		except DaemonException as e:
> +			e.set_atom_url(url)
> +			await self.client.report_err(e.thread_url)

You need to handle a non-DaemonException too, because 'get_atom_urls()'
may 'raise' an exception that's not instance of these 3:

    (DaemonException, OperationalError, DatabaseError)

Also, please report both, @atom_url and @thread_url. Something like this
(just a semi pseudo-code):

         atom_url = None
         try:

                 for atom_url in self.db.get_atom_urls():
                         await self.__handle_atom_url(atom_url)

         except (OperationalError, DatabaseError) as e:
                 await self.handle_db_error(e)
         except (TelegramDaemonException) as e:
                 e.set_atom_url(atom_url)
                 await self.client.report_err(e)
         except e_orig:
                 e = DaemonException()
                 e.set_atom_url(atom_url)
                 e.set_thread_url(None)
                 e.set_message(e_orig)
                 await self.client.report_err(e)


// The report_err side

class Client(whatnot):

         def report_err(self, e: DaemonException):
                 #
                 # Send atom_url, thread_url, and orig exception msg
                 #

Maybe you're aware of a better design than this. If so, send the better
version.

-- 
Ammar Faizi

Re: [PATCH v1 08/13] feat(discord): Implement the report_err()

[Ammar Faizi]

On 1/3/23 1:36 PM, Muhammad Rizki wrote:
> --- a/daemon/dscord/mailer/listener.py
> +++ b/daemon/dscord/mailer/listener.py
> @@ -51,8 +51,13 @@ class Listener:
>   
>   	async def __run(self):
>   		self.logger.info("Running...")
> -		for url in self.db.get_atom_urls():
> -			await self.__handle_atom_url(url)
> +		url = None
> +
> +		try:
> +			for url in self.db.get_atom_urls():
> +				await self.__handle_atom_url(url)
> +		except:
> +			await self.client.report_err(url)

Why don't you implement the same custom exception for Discord?

The exception issue when reporting the corresponding URL also exists on
the Discord daemon too. Yes? No?

-- 
Ammar Faizi

Re: [PATCH v1 08/13] feat(discord): Implement the report_err()

[Ammar Faizi]

On 1/3/23 2:14 PM, Ammar Faizi wrote:
> Why don't you implement the same custom exception for Discord?
> 
> The exception issue when reporting the corresponding URL also exists on
> the Discord daemon too. Yes? No?

OK, just took a look at patch #12, and you implemented it there.
But why should we implement the broken report_err() first, then
the fixed report_err() with DaemonException?

I think implementing the old broken exception handling design is
not needed, just go with the DaemonException pattern we agree on.

-- 
Ammar Faizi

Re: [PATCH v1 11/13] feat(discord): Handle MySQL connection error in the mail listener

[Ammar Faizi]

On 1/3/23 1:36 PM, Muhammad Rizki wrote:
> diff --git a/daemon/dscord/mailer/listener.py b/daemon/dscord/mailer/listener.py
> index ec58f6a..2bde0f0 100644
> --- a/daemon/dscord/mailer/listener.py
> +++ b/daemon/dscord/mailer/listener.py
> @@ -6,6 +6,7 @@
>   
>   import asyncio
>   import re
> +from mysql.connector.errors import OperationalError, DatabaseError
>   from apscheduler.schedulers.asyncio import AsyncIOScheduler
>   from discord import File
>   from discord import Message
> @@ -74,6 +75,8 @@ class Listener:
>   		try:
>   			for url in self.db.get_atom_urls():
>   				await self.__handle_atom_url(url)
> +		except (OperationalError, DatabaseError) as e:
> +			await self.handle_db_error(e)
This is trivial and still very relevant with patch #10, please squash
this into patch #10.

-- 
Ammar Faizi

Re: [PATCH v1 00/13] Improvements, refactors, and features

[Ammar Faizi]

On 1/3/23 1:36 PM, Muhammad Rizki wrote:
> Please give it a test and give me more comments or suggestion, thanks!

I left several comments to the patches. Please take a look. I skip the
testing for this series. I'll do it in v2 (maybe).

One more on the commit message convention:

> Muhammad Rizki (13):
>    refactor(discord): Remove unnecessary try/except block
>    refactor(telegram): Move report_err() to the DaemonClient() class
>    refactor(telegram)!: Ensure the Telegram bot has been started
>    feat: add DaemonException() class
>    feat(telegram): Implement the DaemonException() class
>    refactor(telegram/mailer)!: Remove unnecessary try/except block
>    feat(discord): add report_err() for the Discord bot
>    feat(discord): Implement the report_err()
>    feat(discord/database): Add ping() method
>    feat(discord): Add handle_db_error in the listener.py
>    feat(discord): Handle MySQL connection error in the mail listener
>    feat(discord): Implement the DaemonException for handle error
>    refactor(telegram)!: Rename the Telegram bot custom client

Please don't use 'feat()' and 'refactor()' as the prefix. I never
announced a commit message convention like that for this project.

-- 
Ammar Faizi

Re: [PATCH v1 03/13] refactor(telegram)!: Ensure the Telegram bot has been started

[Muhammad Rizki]

On 03/01/2023 13.54, Ammar Faizi wrote:
> On 1/3/23 1:36 PM, Muhammad Rizki wrote:
>> @@ -58,9 +59,11 @@ def main():
>>           scraper=Scraper(),
>>           mutexes=BotMutexes()
>>       )
>> -    sched.start()
>> +
>> +    client.start()
>>       bot.run()
>> -    client.run()
>> +    idle()
>> +    client.stop()
> 
> I agree to have an 'idle()' call, but it addresses a different issue
> w.r.t. the patch subject. Please split the 'idle()' patch into a
> separate patch.
> 

No, we should use this method rather than `client.run()`. Why? because 
at the previous commit, we use `client.run()` after the `bot.run()` 
which is the `bot` here is running first before the Telegram bot is 
started/connect to the Telegram MTProto API, previous commit:

	sched.start()
	
	bot.run() 	<- we listen for new emails, if error may
			happen, the listener will send the log file
			which is require to interact with the Telegram
			API.
	
	client.run()	<- The client is late to start/connect to the
			Telegram API.


So, my solutions is to use this method:

	client.start()	<- We start/connect to the Telegram API first.

	bot.run()	<- Then, we listen for new emails, if error may
			happen, the bot is already connected to the
			Telegram API to send the log file.
	
	idle()		<- We idle the bot to keep interact with the
			Telegram API.
	
	client.stop()	<- Stopped when CTRL + C or SIGINT.


What do you think?

> My understanding is that the reason for the 'idle()' call is to make the
> program gracefully exits when it's interrupted (e.g., SIGINT with
> CTRL + C), then you have a chance to call 'client.stop()', which could
> be a necessary cleanup to stop the app.
> 
> Is my understanding correct?
> 

Yes.

Re: [PATCH v1 08/13] feat(discord): Implement the report_err()

[Muhammad Rizki]

On 03/01/2023 14.14, Ammar Faizi wrote:
> On 1/3/23 1:36 PM, Muhammad Rizki wrote:
>> --- a/daemon/dscord/mailer/listener.py
>> +++ b/daemon/dscord/mailer/listener.py
>> @@ -51,8 +51,13 @@ class Listener:
>>       async def __run(self):
>>           self.logger.info("Running...")
>> -        for url in self.db.get_atom_urls():
>> -            await self.__handle_atom_url(url)
>> +        url = None
>> +
>> +        try:
>> +            for url in self.db.get_atom_urls():
>> +                await self.__handle_atom_url(url)
>> +        except:
>> +            await self.client.report_err(url)
> 
> Why don't you implement the same custom exception for Discord?
> 
> The exception issue when reporting the corresponding URL also exists on
> the Discord daemon too. Yes? No?
> 

I already added the same custom exception, DaemonException() is reusable 
error class for both Telegram and Discord. The location is in the 
exceptions.py in the daemon directory.

Re: [PATCH v1 03/13] refactor(telegram)!: Ensure the Telegram bot has been started

[Ammar Faizi]

On 1/3/23 7:17 PM, Muhammad Rizki wrote:
> No, we should use this method rather than `client.run()`. Why? because
> at the previous commit, we use `client.run()` after the `bot.run()`
> which is the `bot` here is running first before the Telegram bot is
> started/connect to the Telegram MTProto API, previous commit:
> 
> 	sched.start()
> 	
> 	bot.run() 	<- we listen for new emails, if error may
> 			happen, the listener will send the log file
> 			which is require to interact with the Telegram
> 			API.
> 	
> 	client.run()	<- The client is late to start/connect to the
> 			Telegram API.

What does this call ordering have to do with 'idle()'?

I said this:

On 03/01/2023 13.54, Ammar Faizi wrote:
> I agree to have an 'idle()' call, but it addresses a different issue
> w.r.t. the patch subject. Please split the 'idle()' patch into a
> separate patch.

-- 
Ammar Faizi

Re: [PATCH v1 08/13] feat(discord): Implement the report_err()

[Ammar Faizi]

On 1/3/23 7:20 PM, Muhammad Rizki wrote:
> I already added the same custom exception, DaemonException() is
> reusable error class for both Telegram and Discord. The location is
> in the exceptions.py in the daemon directory.

[ Sending this again because it seems you didn't care with my next reply ]

OK, just took a look at patch #12, and you implemented it there.
But why should we implement the broken report_err() first, then
the fixed report_err() with DaemonException?

I think implementing the old broken exception handling design is
not needed, just go with the DaemonException pattern we agree on.

-- 
Ammar Faizi

Re: [PATCH v1 03/13] refactor(telegram)!: Ensure the Telegram bot has been started

[Alviro Iskandar Setiawan]

On Wed, Jan 4, 2023 at 5:11 AM Ammar Faizi <ammarfaizi2@gnuweeb.org> wrote:
> On 1/3/23 7:17 PM, Muhammad Rizki wrote:
> > No, we should use this method rather than `client.run()`. Why? because
> > at the previous commit, we use `client.run()` after the `bot.run()`
> > which is the `bot` here is running first before the Telegram bot is
> > started/connect to the Telegram MTProto API, previous commit:
> >
> >       sched.start()
> >
> >       bot.run()       <- we listen for new emails, if error may
> >                       happen, the listener will send the log file
> >                       which is require to interact with the Telegram
> >                       API.
> >
> >       client.run()    <- The client is late to start/connect to the
> >                       Telegram API.
>
> What does this call ordering have to do with 'idle()'?

because the subsequent code calls client.stop(), what if you don't
call client.stop()? but

    bot.run() # internally calls sched.start()

Re: [PATCH v1 03/13] refactor(telegram)!: Ensure the Telegram bot has been started

[Ammar Faizi]

On 1/4/23 5:27 AM, Alviro Iskandar Setiawan wrote:
> because the subsequent code calls client.stop(), what if you don't
> call client.stop()? but
> 
>      bot.run() # internally calls sched.start()

Right, that's what I'm talking about.

-- 
Ammar Faizi

Re: [PATCH v1 03/13] refactor(telegram)!: Ensure the Telegram bot has been started

[Muhammad Rizki]

On 04/01/2023 05.27, Alviro Iskandar Setiawan wrote:
> On Wed, Jan 4, 2023 at 5:11 AM Ammar Faizi <ammarfaizi2@gnuweeb.org> wrote:
>> On 1/3/23 7:17 PM, Muhammad Rizki wrote:
>>> No, we should use this method rather than `client.run()`. Why? because
>>> at the previous commit, we use `client.run()` after the `bot.run()`
>>> which is the `bot` here is running first before the Telegram bot is
>>> started/connect to the Telegram MTProto API, previous commit:
>>>
>>>        sched.start()
>>>
>>>        bot.run()       <- we listen for new emails, if error may
>>>                        happen, the listener will send the log file
>>>                        which is require to interact with the Telegram
>>>                        API.
>>>
>>>        client.run()    <- The client is late to start/connect to the
>>>                        Telegram API.
>>
>> What does this call ordering have to do with 'idle()'?
> 
> because the subsequent code calls client.stop(), what if you don't
> call client.stop()? but
> 
>      bot.run() # internally calls sched.start()

How about I do the refactoring again like this?

	async def run(self): 	# this is Bot() class
		client.start()
		
		self.runner = self.sched.add_job(
			func=self.__run,
			misfire_grace_time=None,
			max_instances=1
		)

		self.sched.start()
		idle()
		client.stop()

So, in the main file just call it `bot.run()`. What do you think?

Re: [PATCH v1 03/13] refactor(telegram)!: Ensure the Telegram bot has been started

[Alviro Iskandar Setiawan]

On Wed, Jan 4, 2023 at 1:31 PM Muhammad Rizki wrote:
>         async def run(self):    # this is Bot() class
>                 client.start()
>
>                 self.runner = self.sched.add_job(
>                         func=self.__run,
>                         misfire_grace_time=None,
>                         max_instances=1
>                 )
>
>                 self.sched.start()
>                 idle()
>                 client.stop()
>
> So, in the main file just call it `bot.run()`. What do you think?

Looks saner to me. With `self.client.xxxx()`.

Acked-by: Alviro Iskandar Setiawan <alviro.iskandar@gnuweeb.org>

Re: [PATCH v1 03/13] refactor(telegram)!: Ensure the Telegram bot has been started

[Muhammad Rizki]

On 04/01/2023 13.39, Alviro Iskandar Setiawan wrote:
> On Wed, Jan 4, 2023 at 1:31 PM Muhammad Rizki wrote:
>>          async def run(self):    # this is Bot() class
>>                  client.start()
>>
>>                  self.runner = self.sched.add_job(
>>                          func=self.__run,
>>                          misfire_grace_time=None,
>>                          max_instances=1
>>                  )
>>
>>                  self.sched.start()
>>                  idle()
>>                  client.stop()
>>
>> So, in the main file just call it `bot.run()`. What do you think?
> 
> Looks saner to me. With `self.client.xxxx()`.

Ah yes my bad, I'll refactor it with amend then.

> 
> Acked-by: Alviro Iskandar Setiawan <alviro.iskandar@gnuweeb.org>

Re: [PATCH v1 03/13] refactor(telegram)!: Ensure the Telegram bot has been started

[Ammar Faizi]

On Wed, Jan 04, 2023 at 01:39:35PM +0700, Alviro Iskandar Setiawan wrote:
> On Wed, Jan 4, 2023 at 1:31 PM Muhammad Rizki wrote:
> >         async def run(self):    # this is Bot() class
> >                 client.start()
> >
> >                 self.runner = self.sched.add_job(
> >                         func=self.__run,
> >                         misfire_grace_time=None,
> >                         max_instances=1
> >                 )
> >
> >                 self.sched.start()
> >                 idle()
> >                 client.stop()
> >
> > So, in the main file just call it `bot.run()`. What do you think?
> 
> Looks saner to me. With `self.client.xxxx()`.

I'm fine with this, but from I understand, there are two different
issues:

  1. Wrong ordering between 'client.start()' and 'sched.start()'.
     'client.start()' has to be called first because if an error
     happens before the report functional gets started, we won't get
     the report.

  2. Missing 'client.stop()' (IOW, missing graceful exit handler).

Calling 'client.start()' before 'sched.start()' fixes the former issue.
But the 'idle()' call fixes the later issue. That's why I'm asking to
have a separate patch for it. See what I'm saying? Or did I miss
something very clear here?

-- 
Ammar Faizi

Re: [PATCH v1 03/13] refactor(telegram)!: Ensure the Telegram bot has been started

[Alviro Iskandar Setiawan]

On Wed, Jan 4, 2023 at 1:50 PM Ammar Faizi wrote:
> I'm fine with this, but from I understand, there are two different
> issues:
>
>   1. Wrong ordering between 'client.start()' and 'sched.start()'.
>      'client.start()' has to be called first because if an error
>      happens before the report functional gets started, we won't get
>      the report.
>
>   2. Missing 'client.stop()' (IOW, missing graceful exit handler).
>
> Calling 'client.start()' before 'sched.start()' fixes the former issue.
> But the 'idle()' call fixes the later issue. That's why I'm asking to
> have a separate patch for it. See what I'm saying? Or did I miss
> something very clear here?

I think we don't actually care with the former issue because it's at
initialization, if the initialization fails, something has gone wrong
at a fatal level and needs to be fixed immediately. No one needs the
file report when it happens. So the subject itself should say:

   telegram: Perform graceful exit when interrupted by a signal

and then the changelog explains the need of calling idle() and
client.stop(). Not the ordering issue.

Re: [PATCH v1 03/13] refactor(telegram)!: Ensure the Telegram bot has been started

[Ammar Faizi]

On Wed, Jan 04, 2023 at 02:02:19PM +0700, Alviro Iskandar Setiawan wrote:
> I think we don't actually care with the former issue because it's at
> initialization, if the initialization fails, something has gone wrong
> at a fatal level and needs to be fixed immediately. No one needs the
> file report when it happens. So the subject itself should say:
> 
>    telegram: Perform graceful exit when interrupted by a signal
> 
> and then the changelog explains the need of calling idle() and
> client.stop(). Not the ordering issue.

Good point. Let's do that.

-- 
Ammar Faizi

Re: [PATCH v1 08/13] feat(discord): Implement the report_err()

[Muhammad Rizki]

On 04/01/2023 05.20, Ammar Faizi wrote:
> On 1/3/23 7:20 PM, Muhammad Rizki wrote:
>> I already added the same custom exception, DaemonException() is
>> reusable error class for both Telegram and Discord. The location is
>> in the exceptions.py in the daemon directory.
> 
> [ Sending this again because it seems you didn't care with my next reply ]
> 
> OK, just took a look at patch #12, and you implemented it there.
> But why should we implement the broken report_err() first, then
> the fixed report_err() with DaemonException?
> 
> I think implementing the old broken exception handling design is
> not needed, just go with the DaemonException pattern we agree on.
> 

I don't understand. Do you mean the new `report_err(self, e: 
DaemonException)`?

Re: [PATCH v1 08/13] feat(discord): Implement the report_err()

[Ammar Faizi]

On 1/4/23 3:21 PM, Muhammad Rizki wrote:> I don't understand. Do you mean the new `report_err(self, e:
> DaemonException)`?

Reporting error without DaemonException is always wrong in our situation
right now because we don't get both @atom_url and @thread_url. This
implementation is partially broken.

You even said it yourself in the code:

    "# this statement will be changed in the next commit"

because you know that we don't want to do this. Then why do this? Just
do the correct error reporting in the same patch, no need to implement
the wrong first.

-- 
Ammar Faizi

Re: [PATCH v1 08/13] feat(discord): Implement the report_err()

[Ammar Faizi]

On 1/4/23 3:30 PM, Ammar Faizi wrote:
> Reporting error without DaemonException is always wrong in our situation
> right now because we don't get both @atom_url and @thread_url. This
> implementation is partially broken.
> 
> You even said it yourself in the code:
> 
>     "# this statement will be changed in the next commit"
> 
> because you know that we don't want to do this. Then why do this? Just
> do the correct error reporting in the same patch, no need to implement
> the wrong first.

Side note:
Splitting changes into multiple parts is fine, but no leaving broken bits
in the middle.

If you want a preparation patch, for example writing a big function
helper. That patch should HAVE zero effect to the runtime. A preparation
patch should not be doing broken things at runtime.

-- 
Ammar Faizi

Re: [PATCH v1 08/13] feat(discord): Implement the report_err()

[Ammar Faizi]

On 1/4/23 3:37 PM, Ammar Faizi wrote:
> Side note:
> Splitting changes into multiple parts is fine, but no leaving broken bits
> in the middle.
> 
> If you want a preparation patch, for example writing a big function
> helper. That patch should HAVE zero effect to the runtime. A preparation
> patch should not be doing broken things at runtime.

Let me get this clear. I think this is the proper ordering of the patches:

    1. Remove unnecessary try/except.

    2. Create the DaemonException class.

    3. Create the report_err() function.

    4. Implement report_err() and DaemonException() in the try/except
       block.

-- 
Ammar Faizi

Re: [PATCH v1 08/13] feat(discord): Implement the report_err()

[Muhammad Rizki]

On 04/01/2023 05.20, Ammar Faizi wrote:
> On 1/3/23 7:20 PM, Muhammad Rizki wrote:
>> I already added the same custom exception, DaemonException() is
>> reusable error class for both Telegram and Discord. The location is
>> in the exceptions.py in the daemon directory.
> 
> [ Sending this again because it seems you didn't care with my next reply ]
> 
> OK, just took a look at patch #12, and you implemented it there.
> But why should we implement the broken report_err() first, then
> the fixed report_err() with DaemonException?
> 
> I think implementing the old broken exception handling design is
> not needed, just go with the DaemonException pattern we agree on.
> 

I still don't understand what's your mean, you mean this?

	atom_url = None
         try:

                 for atom_url in self.db.get_atom_urls():
                         await self.__handle_atom_url(atom_url)

         except (OperationalError, DatabaseError) as e:
                 await self.handle_db_error(e)
         except (TelegramDaemonException) as e:
                 e.set_atom_url(atom_url)
                 await self.client.report_err(e)
         except e_orig:
                 e = DaemonException()
                 e.set_atom_url(atom_url)
                 e.set_thread_url(None)
                 e.set_message(e_orig)
                 await self.client.report_err(e)


	def report_err(self, e: DaemonException):
         	pass

Re: [PATCH v1 08/13] feat(discord): Implement the report_err()

[Ammar Faizi]

On Sat, Jan 07, 2023 at 01:10:24PM +0700, Muhammad Rizki wrote:
> I still don't understand what's your mean, you mean this?

Yes, with report_err() being complete. Please read other replies, I
have sent multiple emails with very clear explanation after that one you
replied, please read them too.

-- 
Ammar Faizi

Re: [PATCH v1 08/13] feat(discord): Implement the report_err()

[Alviro Iskandar Setiawan]

On Sat, Jan 7, 2023 at 1:18 PM Ammar Faizi wrote:
> On Sat, Jan 07, 2023 at 01:10:24PM +0700, Muhammad Rizki wrote:
> > I still don't understand what's your mean, you mean this?
>
> Yes, with report_err() being complete. Please read other replies, I
> have sent multiple emails with very clear explanation after that one you
> replied, please read them too.

It seems to me that whenever you reply to your own email, Rizki never
reads it. Maybe something goes wrong with his MUA.

Re: [PATCH v1 08/13] feat(discord): Implement the report_err()

[Ammar Faizi]

On Sat, Jan 07, 2023 at 01:22:14PM +0700, Alviro Iskandar Setiawan wrote:
> On Sat, Jan 7, 2023 at 1:18 PM Ammar Faizi wrote:
> > On Sat, Jan 07, 2023 at 01:10:24PM +0700, Muhammad Rizki wrote:
> > > I still don't understand what's your mean, you mean this?
> >
> > Yes, with report_err() being complete. Please read other replies, I
> > have sent multiple emails with very clear explanation after that one you
> > replied, please read them too.
> 
> It seems to me that whenever you reply to your own email, Rizki never
> reads it. Maybe something goes wrong with his MUA.

Yeah, I feel so too. Rizki, please fix your mail client (or maybe your
broken workflow). This is not productive and wasting time since you
missed many replies that I've sent as a follow up reply to my previous
replies.

-- 
Ammar Faizi

Re: [PATCH v1 08/13] feat(discord): Implement the report_err()

[Muhammad Rizki]

On 07/01/2023 13.22, Alviro Iskandar Setiawan wrote:
> On Sat, Jan 7, 2023 at 1:18 PM Ammar Faizi wrote:
>> On Sat, Jan 07, 2023 at 01:10:24PM +0700, Muhammad Rizki wrote:
>>> I still don't understand what's your mean, you mean this?
>>
>> Yes, with report_err() being complete. Please read other replies, I
>> have sent multiple emails with very clear explanation after that one you
>> replied, please read them too.
> 
> It seems to me that whenever you reply to your own email, Rizki never
> reads it. Maybe something goes wrong with his MUA.

It was sent to me, and even though I didn't understand it at first, I 
didn't reply. But now I understand it. Sir Ammar wants to add the 
correct report_err(e: DaemonException) for the Discord bot, as he 
agrees, and there's no need to add the wrong one.

Re: [PATCH v1 08/13] feat(discord): Implement the report_err()

[Ammar Faizi]

On Sat, Jan 07, 2023 at 02:02:26PM +0700, Muhammad Rizki wrote:
> It was sent to me, and even though I didn't understand it at first, I didn't
> reply. But now I understand it. Sir Ammar wants to add the correct
> report_err(e: DaemonException) for the Discord bot, as he agrees, and
> there's no need to add the wrong one.

Nice to know that, go on.

-- 
Ammar Faizi