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