Re: Next release

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