Re: Next release

From: Murtuza Zabuawala <murtuza(dot)zabuawala(at)enterprisedb(dot)com>
To: Dave Page <dpage(at)pgadmin(dot)org>
Cc: Harshal Dhumal <harshal(dot)dhumal(at)enterprisedb(dot)com>, Surinder Kumar <surinder(dot)kumar(at)enterprisedb(dot)com>, Ashesh Vashi <ashesh(dot)vashi(at)enterprisedb(dot)com>, pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: Next release
Date: 2017-11-17 03:48:19
Message-ID: CAKKotZRs7cOxgWmaEmwJkZq00jJMFCzjvwSYvODj6ZFtnQHP9g@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

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
>

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Murtuza Zabuawala 2017-11-17 06:46:50 Re: [pgAdmin4][Patch]: To fix issues in Boolean editor
Previous Message Dave Page 2017-11-16 16:33:17 Re: [pgAdmin4][Patch]: To fix issues in Boolean editor