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. -- Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company
