Re: Next release

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.

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