On Fri, Nov 17, 2017 at 10:04 AM, Surinder Kumar < surinder.ku...@enterprisedb.com> wrote:
> Hi > > On Fri, Nov 17, 2017 at 2:46 PM, Dave Page <dp...@pgadmin.org> wrote: > >> Hi >> >> The entire point of this patch was to allow an older version of pgAdmin >> to run against a newer version of the database without throwing errors like >> this. Of course, it'll only work if we're careful to ensure we don't make >> any backward-incompatible changes, but adding columns such as this change >> does should not cause issues. >> > Since this patch was commited in pgAdmin4.2.0 where it executes > `db_upgrade(app)` > method defined > in `pgadmin/__init__.py` > if and only if current schema version is greater than schema version > entry present in version table. Now this code(to check version) is not > present in 1.5v and it always goes to run ' db_upgrade(app)' and thus it > fails. > > The solution is to comment the line > `db_upgrade(app)` > in o > lder version of pgAdmin so that it will not run migration against newer > version of database and > thus the newer database will work with older code. > I'm not using pgAdmin < 2.0. I was working from head, tested the patch, then reverted it. > >> >> On Fri, Nov 17, 2017 at 3:48 AM, Murtuza Zabuawala < >> murtuza.zabuaw...@enterprisedb.com> wrote: >> >>> Hi Dave, >>> >>> There is no programmatic way of fixing the issue if you have already >>> deleted the new migration file without downgrading via alembic downgrade >>> command. >>> The only way I know is to manually update 'alembic_version' table in >>> pgAdmin4.db to current revision which is 'ef590e979b0d' >>> >>> -- Murtuza >>> >>> >>> On Thu, Nov 16, 2017 at 6:58 PM, Dave Page <dp...@pgadmin.org> wrote: >>> >>>> >>>> >>>> On Fri, Aug 25, 2017 at 9:34 AM, 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 :-( >>>>> >>>> >>>> Hmm, can you check this please? I'm getting the error below when using >>>> a newer DB (that I tested an upgrade with) with an older code version: >>>> >>>> Traceback (most recent call last): >>>> File "/Users/dpage/git/pgadmin4/web/pgAdmin4.py", line 67, in >>>> <module> >>>> app = create_app() >>>> File "/Users/dpage/git/pgadmin4/web/pgadmin/__init__.py", line 303, >>>> in create_app >>>> db_upgrade(app) >>>> File "/Users/dpage/git/pgadmin4/web/pgadmin/setup/db_upgrade.py", >>>> line 25, in db_upgrade >>>> flask_migrate.upgrade(migration_folder) >>>> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packa >>>> ges/flask_migrate/__init__.py", line 244, in upgrade >>>> command.upgrade(config, revision, sql=sql, tag=tag) >>>> File >>>> "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/alembic/command.py", >>>> line 254, in upgrade >>>> script.run_env() >>>> File >>>> "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/alembic/script/base.py", >>>> line 425, in run_env >>>> util.load_python_file(self.dir, 'env.py') >>>> File >>>> "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/alembic/util/pyfiles.py", >>>> line 81, in load_python_file >>>> module = load_module_py(module_id, path) >>>> File >>>> "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/alembic/util/compat.py", >>>> line 141, in load_module_py >>>> mod = imp.load_source(module_id, path, fp) >>>> File >>>> "/Users/dpage/git/pgadmin4/web/pgadmin/setup/../../migrations/env.py", >>>> line 94, in <module> >>>> run_migrations_online() >>>> File >>>> "/Users/dpage/git/pgadmin4/web/pgadmin/setup/../../migrations/env.py", >>>> line 87, in run_migrations_online >>>> context.run_migrations() >>>> File "<string>", line 8, in run_migrations >>>> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packa >>>> ges/alembic/runtime/environment.py", line 836, in run_migrations >>>> self.get_context().run_migrations(**kw) >>>> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packa >>>> ges/alembic/runtime/migration.py", line 321, in run_migrations >>>> for step in self._migrations_fn(heads, self): >>>> File >>>> "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/alembic/command.py", >>>> line 243, in upgrade >>>> return script._upgrade_revs(revision, rev) >>>> File >>>> "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/alembic/script/base.py", >>>> line 338, in _upgrade_revs >>>> for script in reversed(list(revs)) >>>> File >>>> "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/contextlib.py", >>>> line 35, in __exit__ >>>> self.gen.throw(type, value, traceback) >>>> File >>>> "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/alembic/script/base.py", >>>> line 174, in _catch_revision_errors >>>> compat.raise_from_cause(util.CommandError(resolution)) >>>> File >>>> "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/alembic/util/compat.py", >>>> line 205, in raise_from_cause >>>> reraise(type(exception), exception, tb=exc_tb) >>>> File >>>> "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/alembic/script/base.py", >>>> line 143, in _catch_revision_errors >>>> yield >>>> File >>>> "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/alembic/script/base.py", >>>> line 334, in _upgrade_revs >>>> revs = list(revs) >>>> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packa >>>> ges/alembic/script/revision.py", line 645, in _iterate_revisions >>>> requested_lowers = self.get_revisions(lower) >>>> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packa >>>> ges/alembic/script/revision.py", line 299, in get_revisions >>>> return sum([self.get_revisions(id_elem) for id_elem in id_], ()) >>>> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packa >>>> ges/alembic/script/revision.py", line 304, in get_revisions >>>> for rev_id in resolved_id) >>>> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packa >>>> ges/alembic/script/revision.py", line 304, in <genexpr> >>>> for rev_id in resolved_id) >>>> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packa >>>> ges/alembic/script/revision.py", line 362, in _revision_for_ident >>>> resolved_id) >>>> alembic.util.exc.CommandError: Can't locate revision identified by >>>> '02b9dccdcfcb' >>>> >>>> >>>>> >>>>> >>>>>> >>>>>> -- >>>>>> *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 >>>>> >>>> >>>> >>>> >>>> -- >>>> 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 >> > > -- Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company
