Re: Next release

On Fri, Nov 17, 2017 at 10:04 AM, Surinder Kumar <
surinder(dot)kumar(at)enterprisedb(dot)com> wrote:

> Hi
>
> On Fri, Nov 17, 2017 at 2:46 PM, Dave Page <dpage(at)pgadmin(dot)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(dot)zabuawala(at)enterprisedb(dot)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 <dpage(at)pgadmin(dot)org> wrote:
>>>
>>>>
>>>>
>>>> On Fri, Aug 25, 2017 at 9:34 AM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>>>
>>>>>
>>>>>
>>>>> On Fri, Aug 25, 2017 at 9:28 AM, Harshal Dhumal <
>>>>> harshal(dot)dhumal(at)enterprisedb(dot)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 <dpage(at)pgadmin(dot)org> wrote:
>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On Fri, Aug 25, 2017 at 9:15 AM, Surinder Kumar <
>>>>>>> surinder(dot)kumar(at)enterprisedb(dot)com> wrote:
>>>>>>>
>>>>>>>> ​Hi​
>>>>>>>>
>>>>>>>> On Fri, Aug 25, 2017 at 12:21 PM, Ashesh Vashi <
>>>>>>>> ashesh(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>>>>>
>>>>>>>>> On Fri, Aug 25, 2017 at 12:16 PM, Surinder Kumar <
>>>>>>>>> surinder(dot)kumar(at)enterprisedb(dot)com> wrote:
>>>>>>>>>
>>>>>>>>>> Hi
>>>>>>>>>> On Fri, Aug 25, 2017 at 1:03 AM, Dave Page <dpage(at)pgadmin(dot)org>
>>>>>>>>>> wrote:
>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> On Thu, Aug 24, 2017 at 8:28 PM, Harshal Dhumal <
>>>>>>>>>>> harshal(dot)dhumal(at)enterprisedb(dot)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 <dpage(at)pgadmin(dot)org>
>>>>>>>>>>>> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> On Thu, Aug 24, 2017 at 10:36 AM, Surinder Kumar <
>>>>>>>>>>>>> surinder(dot)kumar(at)enterprisedb(dot)com> wrote:
>>>>>>>>>>>>>
>>>>>>>>>>>>>> Hi Dave,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On Thu, Aug 24, 2017 at 2:28 PM, Dave Page <dpage(at)pgadmin(dot)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