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