Re: Next release

One more thing that this will only work for future pgAdmin4 versions

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