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. >> >> -- >> 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
