Re: Next release

From: Dave Page <dpage(at)pgadmin(dot)org>
To: Harshal Dhumal <harshal(dot)dhumal(at)enterprisedb(dot)com>
Cc: 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-08-25 08:34:28
Message-ID: CA+OCxowF6hg_sngOywyjy7Y6z41O=3ieAa1PAX=12oPM1=7WMg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

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

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

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Dave Page 2017-08-25 08:44:07 Re: [pgAdmin4][Patch]: Allow user to provide custom SSL certificates and provide .pgpass file
Previous Message Dave Page 2017-08-25 08:33:00 Re: pgAdmin4: Random failure of FTS test cases due to improper random string creation