[pgAdmin][RM-6460]: Need a mechanism to detect a corrupt/broken config DB file.

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

[pgAdmin][RM-6460]: Need a mechanism to detect a corrupt/broken config DB file.

nikhil.mohite
Hi Hackers,

Please find the attached patch for RM-6460:  Need a mechanism to detect a corrupt/broken config DB file.

1. Added checks if all tables added in the model are present in SQLite DB or not.
2. If migrations fail it will backup older file and try migrations with the newly created file. 
(User will get notification on UI for the location of the backup file and newly created.)
3. If the user deleted any table from SQLite DB pgAdmin will not run on the next restart and it will add the missing table list in the logs.

--
Thanks & Regards,
Nikhil Mohite
Software Engineer.
Mob.No: +91-7798364578.

RM_6460.patch (12K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [pgAdmin][RM-6460]: Need a mechanism to detect a corrupt/broken config DB file.

Dave Page-7
Hi

On Thu, Jun 3, 2021 at 7:39 AM Nikhil Mohite <[hidden email]> wrote:
Hi Hackers,

Please find the attached patch for RM-6460:  Need a mechanism to detect a corrupt/broken config DB file.

1. Added checks if all tables added in the model are present in SQLite DB or not.
2. If migrations fail it will backup older file and try migrations with the newly created file. 
(User will get notification on UI for the location of the backup file and newly created.)
3. If the user deleted any table from SQLite DB pgAdmin will not run on the next restart and it will add the missing table list in the logs.

Surely if any tables have been deleted, it'll fail the check in point 1?
 
--
Reply | Threaded
Open this post in threaded view
|

Re: [pgAdmin][RM-6460]: Need a mechanism to detect a corrupt/broken config DB file.

nikhil.mohite
Hi Dave,

On Thu, Jun 3, 2021 at 1:47 PM Dave Page <[hidden email]> wrote:
Hi

On Thu, Jun 3, 2021 at 7:39 AM Nikhil Mohite <[hidden email]> wrote:
Hi Hackers,

Please find the attached patch for RM-6460:  Need a mechanism to detect a corrupt/broken config DB file.

1. Added checks if all tables added in the model are present in SQLite DB or not.
2. If migrations fail it will backup older file and try migrations with the newly created file. 
(User will get notification on UI for the location of the backup file and newly created.)
3. If the user deleted any table from SQLite DB pgAdmin will not run on the next restart and it will add the missing table list in the logs.

Surely if any tables have been deleted, it'll fail the check in point 1?
Yes, but if the user deletes any table while pgAdmin is running then it will fail when the user tries to run pgAdmin next time.
(If we remove the table from the model it will not check particular table is present in DB or not. )
 
--
Regards,
Nikhil Mohite. 
Reply | Threaded
Open this post in threaded view
|

Re: [pgAdmin][RM-6460]: Need a mechanism to detect a corrupt/broken config DB file.

Dave Page-7
Hi

On Thu, Jun 3, 2021 at 10:01 AM Nikhil Mohite <[hidden email]> wrote:
Hi Dave,

On Thu, Jun 3, 2021 at 1:47 PM Dave Page <[hidden email]> wrote:
Hi

On Thu, Jun 3, 2021 at 7:39 AM Nikhil Mohite <[hidden email]> wrote:
Hi Hackers,

Please find the attached patch for RM-6460:  Need a mechanism to detect a corrupt/broken config DB file.

1. Added checks if all tables added in the model are present in SQLite DB or not.
2. If migrations fail it will backup older file and try migrations with the newly created file. 
(User will get notification on UI for the location of the backup file and newly created.)
3. If the user deleted any table from SQLite DB pgAdmin will not run on the next restart and it will add the missing table list in the logs.

Surely if any tables have been deleted, it'll fail the check in point 1?
Yes, but if the user deletes any table while pgAdmin is running then it will fail when the user tries to run pgAdmin next time.

Right - but how is the end result of that different from a failed migration that left a table missing? Either way, the end result is the same, and should be handled the same way; i.e. backup/recreate the DB, then warn the user as soon as the UI is up.

I believe the process is simple (and maybe this is what is done and this is just a language issue - I haven't read the patch in detail):

- On startup, attempt to create the database/run any migrations as appropriate.
- Check that all tables exist, and the schema version is correct.
- If there's a problem, backup the file, create a new one from scratch, and warn the user.

One thing I did notice when skimming the patch - we have this in the code:

+                raise RuntimeError(
+                    'Specified SQLite database file is not valid.')

The *user* didn't specify a SQLite database file (nor do they care that it's SQLite at this point). That (and any other similar messages) should probably be rephrased to say something like:

"The configuration database file is not valid."

Thanks!

 
(If we remove the table from the model it will not check particular table is present in DB or not. )
 
--
Regards,
Nikhil Mohite. 


--
Reply | Threaded
Open this post in threaded view
|

Re: [pgAdmin][RM-6460]: Need a mechanism to detect a corrupt/broken config DB file.

nikhil.mohite
Hi Dave,

On Thu, Jun 3, 2021 at 2:49 PM Dave Page <[hidden email]> wrote:
Hi

On Thu, Jun 3, 2021 at 10:01 AM Nikhil Mohite <[hidden email]> wrote:
Hi Dave,

On Thu, Jun 3, 2021 at 1:47 PM Dave Page <[hidden email]> wrote:
Hi

On Thu, Jun 3, 2021 at 7:39 AM Nikhil Mohite <[hidden email]> wrote:
Hi Hackers,

Please find the attached patch for RM-6460:  Need a mechanism to detect a corrupt/broken config DB file.

1. Added checks if all tables added in the model are present in SQLite DB or not.
2. If migrations fail it will backup older file and try migrations with the newly created file. 
(User will get notification on UI for the location of the backup file and newly created.)
3. If the user deleted any table from SQLite DB pgAdmin will not run on the next restart and it will add the missing table list in the logs.

Surely if any tables have been deleted, it'll fail the check in point 1?
Yes, but if the user deletes any table while pgAdmin is running then it will fail when the user tries to run pgAdmin next time.

Right - but how is the end result of that different from a failed migration that left a table missing? Either way, the end result is the same, and should be handled the same way; i.e. backup/recreate the DB, then warn the user as soon as the UI is up.
Yes - It should be the same in both conditions, I will send an updated patch for this.
 

I believe the process is simple (and maybe this is what is done and this is just a language issue - I haven't read the patch in detail):

- On startup, attempt to create the database/run any migrations as appropriate.
- Check that all tables exist, and the schema version is correct.
- If there's a problem, backup the file, create a new one from scratch, and warn the user.

One thing I did notice when skimming the patch - we have this in the code:

+                raise RuntimeError(
+                    'Specified SQLite database file is not valid.')

The *user* didn't specify a SQLite database file (nor do they care that it's SQLite at this point). That (and any other similar messages) should probably be rephrased to say something like:

"The configuration database file is not valid."

Thanks!

 
(If we remove the table from the model it will not check particular table is present in DB or not. )
 
--
Regards,
Nikhil Mohite. 


--
Regards,
Nikhil Mohite 
Reply | Threaded
Open this post in threaded view
|

Re: [pgAdmin][RM-6460]: Need a mechanism to detect a corrupt/broken config DB file.

nikhil.mohite
Hi Team,

PFA updated patch (v2)
On Thu, Jun 3, 2021 at 3:10 PM Nikhil Mohite <[hidden email]> wrote:
Hi Dave,

On Thu, Jun 3, 2021 at 2:49 PM Dave Page <[hidden email]> wrote:
Hi

On Thu, Jun 3, 2021 at 10:01 AM Nikhil Mohite <[hidden email]> wrote:
Hi Dave,

On Thu, Jun 3, 2021 at 1:47 PM Dave Page <[hidden email]> wrote:
Hi

On Thu, Jun 3, 2021 at 7:39 AM Nikhil Mohite <[hidden email]> wrote:
Hi Hackers,

Please find the attached patch for RM-6460:  Need a mechanism to detect a corrupt/broken config DB file.

1. Added checks if all tables added in the model are present in SQLite DB or not.
2. If migrations fail it will backup older file and try migrations with the newly created file. 
(User will get notification on UI for the location of the backup file and newly created.)
3. If the user deleted any table from SQLite DB pgAdmin will not run on the next restart and it will add the missing table list in the logs.

Surely if any tables have been deleted, it'll fail the check in point 1?
Yes, but if the user deletes any table while pgAdmin is running then it will fail when the user tries to run pgAdmin next time.

Right - but how is the end result of that different from a failed migration that left a table missing? Either way, the end result is the same, and should be handled the same way; i.e. backup/recreate the DB, then warn the user as soon as the UI is up.
Yes - It should be the same in both conditions, I will send an updated patch for this. 
 

I believe the process is simple (and maybe this is what is done and this is just a language issue - I haven't read the patch in detail):

- On startup, attempt to create the database/run any migrations as appropriate.
- Check that all tables exist, and the schema version is correct.
- If there's a problem, backup the file, create a new one from scratch, and warn the user.

One thing I did notice when skimming the patch - we have this in the code:

+                raise RuntimeError(
+                    'Specified SQLite database file is not valid.')

The *user* didn't specify a SQLite database file (nor do they care that it's SQLite at this point). That (and any other similar messages) should probably be rephrased to say something like:

"The configuration database file is not valid." 

Thanks!

 
(If we remove the table from the model it will not check particular table is present in DB or not. )
 
--
Regards,
Nikhil Mohite. 


--
Regards,
Nikhil Mohite 
Regards,
Nikhil Mohite 

RM_6460_v2.patch (13K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [pgAdmin][RM-6460]: Need a mechanism to detect a corrupt/broken config DB file.

Akshay Joshi
Thanks, the patch applied.

On Thu, Jun 3, 2021 at 3:52 PM Nikhil Mohite <[hidden email]> wrote:
Hi Team,

PFA updated patch (v2)
On Thu, Jun 3, 2021 at 3:10 PM Nikhil Mohite <[hidden email]> wrote:
Hi Dave,

On Thu, Jun 3, 2021 at 2:49 PM Dave Page <[hidden email]> wrote:
Hi

On Thu, Jun 3, 2021 at 10:01 AM Nikhil Mohite <[hidden email]> wrote:
Hi Dave,

On Thu, Jun 3, 2021 at 1:47 PM Dave Page <[hidden email]> wrote:
Hi

On Thu, Jun 3, 2021 at 7:39 AM Nikhil Mohite <[hidden email]> wrote:
Hi Hackers,

Please find the attached patch for RM-6460:  Need a mechanism to detect a corrupt/broken config DB file.

1. Added checks if all tables added in the model are present in SQLite DB or not.
2. If migrations fail it will backup older file and try migrations with the newly created file. 
(User will get notification on UI for the location of the backup file and newly created.)
3. If the user deleted any table from SQLite DB pgAdmin will not run on the next restart and it will add the missing table list in the logs.

Surely if any tables have been deleted, it'll fail the check in point 1?
Yes, but if the user deletes any table while pgAdmin is running then it will fail when the user tries to run pgAdmin next time.

Right - but how is the end result of that different from a failed migration that left a table missing? Either way, the end result is the same, and should be handled the same way; i.e. backup/recreate the DB, then warn the user as soon as the UI is up.
Yes - It should be the same in both conditions, I will send an updated patch for this. 
 

I believe the process is simple (and maybe this is what is done and this is just a language issue - I haven't read the patch in detail):

- On startup, attempt to create the database/run any migrations as appropriate.
- Check that all tables exist, and the schema version is correct.
- If there's a problem, backup the file, create a new one from scratch, and warn the user.

One thing I did notice when skimming the patch - we have this in the code:

+                raise RuntimeError(
+                    'Specified SQLite database file is not valid.')

The *user* didn't specify a SQLite database file (nor do they care that it's SQLite at this point). That (and any other similar messages) should probably be rephrased to say something like:

"The configuration database file is not valid." 

Thanks!

 
(If we remove the table from the model it will not check particular table is present in DB or not. )
 
--
Regards,
Nikhil Mohite. 


--
Regards,
Nikhil Mohite 
Regards,
Nikhil Mohite 


--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246