Re: Next release

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.

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