Open Stack

On 4/4/19, 9:37 AM, "Matthew Thode" <mthode at mthode.org> wrote:

>On 19-04-04 10:30:45, Eric K wrote:
>> 
>> 
>> On 4/4/19, 9:17 AM, "Matthew Thode" <mthode at mthode.org> wrote:
>> 
>> >On 19-04-04 10:11:04, Eric K wrote:
>> >> On 4/4/19, 7:22 AM, "Clark Boylan" <cboylan at sapwetik.org> wrote:
>> >> 
>> >> >On Thu, Apr 4, 2019, at 7:50 AM, Matthew Thode wrote:
>> >> >> On 19-04-04 08:32:56, Alex Schultz wrote:
>> >> >> > On Thu, Apr 4, 2019 at 7:58 AM Matthew Thode <mthode at mthode.org>
>> >> >>wrote:
>> >> >> > >
>> >> >> > > On 19-04-04 01:20:18, Eric K wrote:
>> >> >> > > > Requesting exception to remove a requirement to resolve a
>> >> >>packaging issue.
>> >> >> > > >
>> >> >> > > > The package is needed only for an experimental feature, but
>> >>causes
>> >> >> > > > packaging issue on RHEL/CentOS 7. Propose to remove from
>> >> >>requirements and
>> >> >> > > > require manual install to enable the experimental feature.
>> >> >> > > >
>> >> >> > > > https://review.openstack.org/#/c/649772/
>> >> >> > > >
>> >> >> > >
>> >> >> > > I imagine psycopg2 is one of many dependencies that are not
>>fully
>> >> >> > > compliant with our upper-constraints (and probably a bunch of
>> >> >>projects
>> >> >> > > global-constraints).  Is it possible to lower the version
>> >>required
>> >> >>(be
>> >> >> > > compatible with 2.6 or whatever is in centos)?  The
>>requirements
>> >> >>project
>> >> >> > > itself sets no lower bound for psycopg2.
>> >> >> > >
>> >> >> > 
>> >> >> > Centos7 has 2.5. I had asked about getting 2.7 in RDO but if
>>it's
>> >> >> > optional it would be nice for it not to be included by default.
>>We
>> >>hit
>> >> >> > the issue in puppet-congress so we disabled out actual test
>>jobs[0]
>> >> >> > because congress currently doesn't work on ubuntu[1] and was
>> >>broken on
>> >> >> > centos[2] by this new requirement.
>> >> >> > 
>> >> >> > [0] https://review.openstack.org/#/c/649439/
>> >> >> > [1] 
>> >> 
>> 
>>>>>>http://logs.openstack.org/33/647333/1/check/puppet-openstack-beaker-u
>>>>>>bu
>> >>>>nt
>> >> >>u-bionic/2cfe5a4/job-output.txt.gz#_2019-04-02_10_48_17_614150
>> >> >> > [2] 
>> >> 
>> 
>>>>>>http://logs.openstack.org/33/647333/1/check/puppet-openstack-beaker-c
>>>>>>en
>> >>>>to
>> >> >>s-7/c10fb99/logs/syslog.txt.gz#_Apr_02_10_47_47
>> >> >> > 
>> >> >> 
>> >> >> extra_requires may work for you (makes postgres support optional)
>> >> >> 
>> >> 
>> 
>>>>>>https://setuptools.readthedocs.io/en/latest/setuptools.html#declaring
>>>>>>-e
>> >>>>xt
>> >> >>ras-optional-features-with-their-own-dependencies
>> >> >> 
>> >> >> not sur eif pbr supports it though.
>> >> >
>> >> >PBR should support it. Here is an example from swiftclient
>>installing
>> >> >keystoneclient as an extra [3].
>> >> >
>> >> >[3] 
>> >> 
>> 
>>>>>https://git.openstack.org/cgit/openstack/python-swiftclient/tree/setup
>>>>>.c
>> >>>fg
>> >> >#n36
>> >> >
>> >> Thank you all!
>> >> 
>> >> 
>> >> Unfortunately the code relies on the psycopg2.sql module for query
>> >> composition, which is only available on 2.7 and up. I'm also working
>>on
>> >>a
>> >> future patch to remove the use of the psycopg2.sql module and also
>>lower
>> >> the psycopg2 version required, but I'm thinking considering the stein
>> >> deadline a straightforward removal of psycopg2 can make sense.
>> >> 
>> >> I have fixed the unit test failure I overlooked and everything
>>should be
>> >> passing now [4]
>> >> 
>> >> And thank you for the extra_requires suggestion. It would probably
>>take
>> >>me
>> >> a little while and a few tries to get it right. Again considering the
>> >> deadline I'd rather just go with the current patch [4] if it's
>> >>acceptable.
>> >> Since the feature that requires psycopg2 is experimental I think
>> >>requiring
>> >> manual install of psycopg2 is ok.
>> >> 
>> >> Alternatively, if it seems better to delay Congress final RC or wait
>>to
>> >> fix this issue in X.0.1 we can do that too.
>> >> 
>> >> [4] https://review.openstack.org/#/c/649772/
>> >> 
>> >
>> >The problem is that you are still requiring postgres support via some
>> >code path, so that means that it's required, either statically or via
>> >extra requires.  I don't think removing it is an appropriate solution.
>> >
>> >-- 
>> >Matthew Thode
>> I see. psycopg2 is required only if the experimental feature (json
>> ingester) is explicitly enabled via feature flag in conf file. Otherwise
>> there is no import.
>> 
>> But if that's not okay, I'll do an alternative approach.
>> 
>
>What happens when someone enables that experimental feature without
>having psycopg2 installed because it was not required / in the
>requirements file?
>
>-- 
>Matthew Thode

I assume that was rhetorical =)
Clearly in that case the experimental feature would not work (import
errors in log).

I also submitted an alternative patch [5] which lowers the requirement to
psycopg2>=2.5.1 but does not remove it completely.

[5] https://review.openstack.org/#/c/650100/