Hi On Fri, Aug 25, 2017 at 2:04 PM, Dave Page <dp...@pgadmin.org> wrote:
> > > On Fri, Aug 25, 2017 at 9:28 AM, Harshal Dhumal < > harshal.dhu...@enterprisedb.com> wrote: > >> One more thing that this will only work for future pgAdmin4 versions >> > > > Yeah :-( > > >> >> -- >> *Harshal Dhumal* >> *Sr. Software Engineer* >> >> EnterpriseDB India: http://www.enterprisedb.com >> The Enterprise PostgreSQL Company >> >> On Fri, Aug 25, 2017 at 1:48 PM, Dave Page <dp...@pgadmin.org> wrote: >> >>> >>> >>> On Fri, Aug 25, 2017 at 9:15 AM, Surinder Kumar < >>> surinder.ku...@enterprisedb.com> wrote: >>> >>>> Hi >>>> >>>> On Fri, Aug 25, 2017 at 12:21 PM, Ashesh Vashi < >>>> ashesh.va...@enterprisedb.com> wrote: >>>> >>>>> On Fri, Aug 25, 2017 at 12:16 PM, Surinder Kumar < >>>>> surinder.ku...@enterprisedb.com> wrote: >>>>> >>>>>> Hi >>>>>> On Fri, Aug 25, 2017 at 1:03 AM, Dave Page <dp...@pgadmin.org> wrote: >>>>>> >>>>>>> >>>>>>> >>>>>>> On Thu, Aug 24, 2017 at 8:28 PM, Harshal Dhumal < >>>>>>> harshal.dhu...@enterprisedb.com> wrote: >>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> -- >>>>>>>> *Harshal Dhumal* >>>>>>>> *Sr. Software Engineer* >>>>>>>> >>>>>>>> EnterpriseDB India: http://www.enterprisedb.com >>>>>>>> The Enterprise PostgreSQL Company >>>>>>>> >>>>>>>> On Thu, Aug 24, 2017 at 9:44 PM, Dave Page <dp...@pgadmin.org> >>>>>>>> wrote: >>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> On Thu, Aug 24, 2017 at 10:36 AM, Surinder Kumar < >>>>>>>>> surinder.ku...@enterprisedb.com> wrote: >>>>>>>>> >>>>>>>>>> Hi Dave, >>>>>>>>>> >>>>>>>>>> On Thu, Aug 24, 2017 at 2:28 PM, Dave Page <dp...@pgadmin.org> >>>>>>>>>> wrote: >>>>>>>>>> >>>>>>>>>>> Anyone object to doing a release on 14th September, wrapping the >>>>>>>>>>> code on Monday 11th? This seems like the best option for our QA >>>>>>>>>>> folks who >>>>>>>>>>> will be off for EID somewhen in the two weeks before. >>>>>>>>>>> >>>>>>>>>>> Assuming not, should this be 1.7 or 2.0? >>>>>>>>>>> >>>>>>>>>>> If we go with 2.0, it'll be for "safety" given the proposed >>>>>>>>>>> changes to path management to allow both server and desktop modes >>>>>>>>>>> to work >>>>>>>>>>> out of the box on Linux. >>>>>>>>>>> >>>>>>>>>>> If we do that, we also need to ensure that any changes to the >>>>>>>>>>> config database are backwards compatible, as a 2.0 release would be >>>>>>>>>>> a >>>>>>>>>>> side-by-side installation. Surinder; was it you that had looked >>>>>>>>>>> into that? >>>>>>>>>>> >>>>>>>>>> I had looked into this and here are my findings: >>>>>>>>>> 1. If we are using newer version of pgAdmin and the go back to >>>>>>>>>> older version of pgAdmin, then on running `python pgAdmin4.py`. the >>>>>>>>>> flask-migrate(Alembic) try to perform downgrade by one step only(ie. >>>>>>>>>> it can >>>>>>>>>> switch back to one migration only when we run `python >>>>>>>>>> pgAdmin4.py`). But >>>>>>>>>> we have multiple database revisions to be migrated. So migration >>>>>>>>>> fails here. >>>>>>>>>> >>>>>>>>>> 2. When Alebmic downgrade is performed by one step, it looks for >>>>>>>>>> downgrade function in that specific database revision, but in our >>>>>>>>>> code we >>>>>>>>>> didn't written downgrade function. But if we have written downgrade >>>>>>>>>> statement, still there is an issue: >>>>>>>>>> ie. If we add a new column to a table xyz using ALTER statement >>>>>>>>>> like: >>>>>>>>>> >>>>>>>>>> ``` >>>>>>>>>> >>>>>>>>>> def upgrade(): >>>>>>>>>> >>>>>>>>>> verison = get_version() >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> db.engine.execute( >>>>>>>>>> >>>>>>>>>> 'ALTER TABLE server ADD COLUMN hostaddr TEXT(1024)' >>>>>>>>>> >>>>>>>>>> ) >>>>>>>>>> >>>>>>>>>> def downgrade(): >>>>>>>>>> >>>>>>>>>> pass >>>>>>>>>> ``` >>>>>>>>>> then on downgrade it executes `downgrade` method, so downgrade >>>>>>>>>> should have code like >>>>>>>>>> `ALTER TABLE server DROP COLUMN hostaddr ` >>>>>>>>>> but in sqlite DROP COLUMN statements don't work. >>>>>>>>>> So, this is a an issue with Sqlite database. However, an >>>>>>>>>> alternative way is also given. Here is link >>>>>>>>>> <https://stackoverflow.com/questions/5938048/delete-column-from-sqlite-table> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> Still, I didn't find any other solution on upgrading/downgrading >>>>>>>>>> database revisions without errors. >>>>>>>>>> It is an issue with Flask-Migrate(Alembic) plugin. >>>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> Urgh. So I guess the other option is that we version the DB >>>>>>>>> filename as well. The downside of that is that users will want to >>>>>>>>> migrate >>>>>>>>> their settings - which may be awkward as we'll have no real way of >>>>>>>>> knowing >>>>>>>>> where they are. >>>>>>>>> >>>>>>>>> Thoughts? >>>>>>>>> >>>>>>>>> Or should we write our own custom backword migrations? For eg. >>>>>>>> dropping column can be achieved by creating another table excluding >>>>>>>> the >>>>>>>> columns which we want to drop then copy data to new table and then >>>>>>>> drop old >>>>>>>> table and rename new table to old name. And also sqlite database schema >>>>>>>> which we have in pgAdmin4 is small so writing and maintaining custom >>>>>>>> migration won be that hard. >>>>>>>> >>>>>>> >>>>>>> The problem is that we don't want to migrate backwards; we want both >>>>>>> versions to be able to run with the same database (for example, because >>>>>>> you >>>>>>> might have multiple versions installed with the EDB PG installer as I >>>>>>> do on >>>>>>> my laptop). >>>>>>> >>>>>>> Previously, we always made sure our changes were backwards >>>>>>> compatible (e.g. by only adding new columns, never removing or renaming >>>>>>> them), and our home-grown migration code only cared about upgrading the >>>>>>> database to the current version; it wouldn't complain if the database >>>>>>> was >>>>>>> of a newer version. >>>>>>> >>>>>> The code which is responsible to run database migration is >>>>>> `db_upgrade(app)` in `pgadmin/__init__.py` it executes when python >>>>>> server >>>>>> runs `python pgAdmin4.py`, It fails with older version of pgAdmin4(say >>>>>> 1.5) >>>>>> because it cannot find db revision file (revision id stored in table >>>>>> 'alembic_version') in `web/migrations` folder of latest pgAdmin4-1.5 >>>>>> >>>>>> But If we catch this exception like: >>>>>> ``` >>>>>> import alembic >>>>>> try: >>>>>> db_upgrade(app) >>>>>> except alembic.util.exc.CommandError as e: # Handle migration >>>>>> error, I expect this exception will be raised in older version of code. >>>>>> app.logger.info('Failed to run migrations: %s' % str(e)) >>>>>> ``` >>>>>> >>>>>> It will fail to run migrations but exception will be handled and >>>>>> python app server will be started successfully and pgAdmin4 will run with >>>>>> newer database. >>>>>> Or, we should check whether the migration which is about to run >>>>>> against the revision id(stored in table alembic_version) exists or not in >>>>>> `web/migrations`. >>>>>> If it exists then run migration otherwise don't run. >>>>>> >>>>>> This way the same database will work for pgAdmin4-1.5 and pgAdmin4-1.6 >>>>>> But the only problem is that we didn't caught exception >>>>>> `alembic.util.exc.CommandError` in older versions of pgAdmin4. >>>>>> >>>>> As per my understanding, that's not safe, as we may not be able to >>>>> catch some other genuine issues. >>>>> >>>> Yes, that's the possibility. >>>> I discussed this with Harshal and discussed about what other possible >>>> workarounds can work. >>>> So, here is the one: >>>> >>>> We will store the current pgAdmin4 version in sqlite table if we are >>>> not storing. Also, current pgAdmin4 version is assigned to config variable >>>> `APP_VERSION` in config.py >>>> >>>> And when running older pgAdmin4 version with new database, >>>> we will compare `if config. >>>> APP_VERSION >>>> < PGADMIN4_VERSION`, then skip migration, otherwise run. >>>> *For example:* >>>> >>>> if config. >>>> APP_VERSION >>>> >= PGADMIN4_VERSION: # Run migration only when >>>> >>>> db_upgrade(app) # run migration >>>> # Now update PGADMIN4_VERSION in sqlite to latest one >>>> >>> >>> That sounds like a reasonable approach, though for developers I think >>> the pgAdmin version wouldn't necessarily work - we'd need a schema version >>> like we used to have. >>> >> Yes, that's true. We will use config variable ` SETTINGS_SCHEMA_VERSION `. We had previously used this variable and current schema version is stored in `version` table. I will send a patch for the same. > >>> -- >>> Dave Page >>> Blog: http://pgsnake.blogspot.com >>> Twitter: @pgsnake >>> >>> EnterpriseDB UK: http://www.enterprisedb.com >>> The Enterprise PostgreSQL Company >>> >> >> > > > -- > Dave Page > Blog: http://pgsnake.blogspot.com > Twitter: @pgsnake > > EnterpriseDB UK: http://www.enterprisedb.com > The Enterprise PostgreSQL Company >
