Re: Next release

​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

Thanks,
Surinder

> -- Thanks, Ashesh
>
>
>>
>>> --
>>> Dave Page
>>> Blog: http://pgsnake.blogspot.com
>>> Twitter: @pgsnake
>>>
>>> EnterpriseDB UK: http://www.enterprisedb.com
>>> The Enterprise PostgreSQL Company
>>>
>>
>>
>