is this a good way to catch exceptions?

[korenron]

Hello ,
I have a small app that uplaod file to my devices using FTP and update the DB.
it's working
my question is - is the "Exceptions" part good? meaning , how do I know if there are other "problems"? also do I need to use "except" and also "Exception as e"?
or both of them point at the same problem ?

        try:
            for file in Files_Path:
                Upload_Status = 'unknown'
                Upload_Status = upload_script(Unit_IP, file)
        except (ConnectionError, TimeoutError): 
            print('TimeOut error - Unit is offline maybe?')
            Upload_Status = 'Offline'
        except Exception as e:
            print(e)
            print('\x1b[3;30;41m' + f'Error! {Unit_IP}' + '\x1b[0m')
            Notupload += 1
            NotUploadIpList.append(Unit_IP)
            Upload_Status = 'Error'
        except:
            print(e)
            print('Unknown error!!!!!!  ' + Unit_IP)
            Upload_Status = 'Unknown Error'
        else:
            print('\x1b[6;30;42m' + f'Success! {Unit_IP}' + '\x1b[0m') ## no problem detected 
            Upload += 1
        finally:
            mycursor1 = mydb.cursor()
            sql1 = f"update david.david_test set COMMENT = '" + Upload_Status + "',SendTime=NOW() where Unit_ip = '" + Unit_IP + "';"
            mycursor1.execute(sql1)
            mydb.commit()

Thanks ,

[buran]

except Exception as e: will catch every exception apart from ConnectionError and TimeoutError from the previous except block thus lines 14-17 ate redundant. bare except is same as except BaseException. Strictly speaking, if it wasn't like this, on line 15 e would be undefined and raise NameError

[korenron]

OK , thank you.

so if could another error ,not ConnectionError or TimeoutError
is there any way to know it before ?
or I have to write except Exception and see\wait for it ? (let's say it's something I didn't think about )

[deanhystad]

The documentation should tell you what exceptions can be raised when you call a function. You don't have to guess.

[korenron]

OK
Thank you

[buran]

One more thing - once there is error your loop will end and will not continue with next iteration.
And if there is no error else and finally block will be executed only after the loop has ended.
My guess is you want to do all this for each file. It will be better if you create the cursor beforehand. Then have the try/except/else/finally block inside the loop.

[korenron]

good point ,
but I if something went wrong in 1 file - it the same as 10 files for me
so I prefer to know there was an error - and try again next time (it need to be 100%, 70% can cause problems)

but thank you ,

[buran]

good point ,
but I if something went wrong in 1 file - it the same as 10 files for me
so I prefer to know there was an error - and try again next time (it need to be 100%, 70% can cause problems)

but thank you ,

Maybe I wasn't clear.
Let say you have 10 files and there is error on file #7. Then you already have uploaded 6 files and you don't know which files.
Also NotUploadIpList.append(Unit_IP) suggest you want to have list of units that failed, but actually there will be just one and your code will end. You will always have a list with one element. So not sure why you use list in this case.

[korenron]

the list that beeing created when there is a problem is OK
I can see 5-7 IP in that list (on error )

and about the files:
I don't care if 9 out of 10 files are success , I need to know the all proccess went without problem.

Thanks ,

[ndc85430]

You really shouldn't be concatenating strings to put data in SQL statements (line 23) - SQL injection is the problem with that. Whichever database you're using will have placeholder characters that you put in the string and execute takes an additional argument that lets you pass the values you want. See the docs for the library for details on how to do this correctly.