From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on gnuweeb.org X-Spam-Level: X-Spam-Status: No, score=-1.8 required=5.0 tests=ALL_TRUSTED,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,NO_DNS_FOR_FROM, URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6 Received: from [192.168.1.2] (unknown [101.128.126.183]) by gnuweeb.org (Postfix) with ESMTPSA id ACF8A804FD; Fri, 28 Oct 2022 18:52:56 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=gnuweeb.org; s=default; t=1666983177; bh=z77RxfO5nhSwQ5HFMnlUXjNkGLK+Yb9GNKU0O1LGxqI=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=q5hTC3k1Pz1ARGZaSIF1jMgaYS8DgtcFeNfeZC3+dquC3lL643O4Du90cMuhxAAQG 2Bg3X/ke71WYAq4HK4txaQUD6+I3q+phVBtHxSd4i2uvIu08xtTEwS/HqVeDpMN5ab idZuFbQ9/AOMzsW8QDEW6MlBmMbT1mVmSkcl8BWXUh9Zi9ywG0qRa7urYIb4k7cK0R jPYQSC/wr70ZpDmQvZIfyaaPGYPFfp6W8HtV5BsnOS6pD+LUysBt50BEGvjeg8nNiE FvmiqlBFvqPAv05kgUl1PKK719mIGhYE8IrQMNmdxopR67tlzqlp9Yo3aZQ1uNmhTg sUP0IsRpNvx7g== Message-ID: <9ecfe8ab-fa5a-d01a-0b68-bc639f14888f@gnuweeb.org> Date: Sat, 29 Oct 2022 01:52:53 +0700 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.2.2 Subject: Re: [PATCH v1 2/2] daemon: telegram: Handle MySQL error Content-Language: en-US To: Alviro Iskandar Setiawan Cc: Ammar Faizi , GNU/Weeb Mailing List References: <20221027150823.601914-1-ammarfaizi2@gnuweeb.org> <20221027150823.601914-3-ammarfaizi2@gnuweeb.org> <1d500d37-b11b-75fd-38e5-d7f8e0a9b1d4@gnuweeb.org> <3a79a587-ddee-9e25-2ac5-b573938b44a9@gnuweeb.org> <043f55c3-67d8-9130-aca4-73c59926d2af@gnuweeb.org> <63aa1b63-f7e2-8da3-b16d-0c7e1045d697@gnuweeb.org> From: Muhammad Rizki In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit List-Id: On 29/10/22 01.26, Alviro Iskandar Setiawan wrote: > On Sat, Oct 29, 2022 at 1:10 AM Muhammad Rizki wrote: >> On 28/10/22 23.46, Alviro Iskandar Setiawan wrote: >>> Even this part is also problematic, if you hit an error before >>> utils.remove_patch(), the patch file won't get removed because >>> exception will stop the current execution and throw the exception to >>> the previous call stack. >> >> You are right. So we just check every value from the DB method and if >> it's None just return like goto? > > You don't use goto in Python. In an OOP style, the cleanup usually > happens in the destructor, or in a "try with finally" statement. > >>> >>> I hate try and catch style, because it's very easy to miss an error >>> like this. I prefer the way C handles error manually via return value >>> like: >>> >>> ret = func_call(); >>> if (ret < 0) >>> goto clean_up; >>> >> >> I've improved the remove_patch() earlier using the glob UNIX style path >> to check if all temp dirs is exists then remove them all. What do you think? > > It doesn't address the issue. You still don't understand the > underlying issue behind your remove_patch() placement. > > You have this: > > for d, f in files: > await m.reply_document(f"{d}/{f}", file_name=f) > await asyncio.sleep(1) > > utils.remove_patch(files) > > What happens if you follow that for loop, then m.reply_document() > throws an exception? It will throw an exception before executing the remove_patch(), but, the problem is not "if m.reply_document() throw an exception", but the files variable from the utils.create_template() is the problem if throw an exception, I think it will be throw a local variable error. Here is my improved remove_patch(): def remove_patch( tmp: Union[str, list] = None, platform: Platform = Platform.TELEGRAM ): if isinstance(tmp, str): return shutil.rmtree(tmp) # check if the tmp is None or an empty list or empty str # if they are empty or None then remove using glob UNIX # style path which is if empty will not throw an error if not bool(tmp): platform = platform.TELEGRAM.value store_dir = os.getenv("STORAGE_DIR", "storage") for d in glob.glob(f"{platform}/{store_dir}/*/"): shutil.rmtree(d) return for d,_ in tmp: shutil.rmtree(d) > There are 2 possible solutions, either using a "try with finally" > statement, or using a destructor wrapped in a class. I will do it if this PATCH series is done and will do it in logger PATCH series later. I wait for sir Ammar create the v2 of this series and pull it after this PATCH series is applied to the master branch.