Re: Next release

From: Ashesh Vashi <ashesh(dot)vashi(at)enterprisedb(dot)com>
To: Surinder Kumar <surinder(dot)kumar(at)enterprisedb(dot)com>
Cc: Dave Page <dpage(at)pgadmin(dot)org>, Harshal Dhumal <harshal(dot)dhumal(at)enterprisedb(dot)com>, pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: Next release
Date: 2017-08-25 06:51:03
Message-ID: CAG7mmoyvuKH5SGdajsncmyEJECbPKZBGYT4Jrx=j4exCwoWNkA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

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.

-- Thanks, Ashesh

>
>> --
>> 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 Surinder Kumar 2017-08-25 08:15:11 Re: Next release
Previous Message Surinder Kumar 2017-08-25 06:46:53 Re: Next release