> Is avoiding three lines of code really worth making future cleanup
> harder? Is a three line change really blocking "an approved blueprint
> with ready code"?

Nope.  What's blocking is deciding that that's the right thing to do.
Which we clearly don't have consensus on, based on what's happening in
this thread.

>         global ironic
>         if ironic is None:
>             ironic = importutils.import_module('ironicclient')

I have a pretty strong dislike for this mechanism.  For one thing, I'm
frustrated when I can't use hotkeys to jump to an ironicclient method
because my IDE doesn't recognize that dynamic import.  I have to go look
up the symbol some other way (and hope I'm getting the right one).  To
me (with my bias as a dev rather than a deployer) that's way worse than
having the 704KB python-ironicclient installed on my machine even though
I've never spawned an ironic VM in my life.

It should also be noted that python-ironicclient is in
test-requirements.txt.

Not that my personal preference ought to dictate or even influence what
we decide to do here.  But dynamic import is not the obviously correct
choice.

-efried

On 04/12/2018 03:28 PM, Michael Still wrote:
> I don't understand why you think the alternative is so hard. Here's how
> ironic does it:
> 
>         global ironic
> 
>         if ironic is None:
> 
>             ironic = importutils.import_module('ironicclient')
> 
> 
> Is avoiding three lines of code really worth making future cleanup
> harder? Is a three line change really blocking "an approved blueprint
> with ready code"?
> 
> Michael
> 
> 
> 
> On Thu, Apr 12, 2018 at 10:42 PM, Eric Fried <openst...@fried.cc
> <mailto:openst...@fried.cc>> wrote:
> 
>     +1
> 
>     This sounds reasonable to me.  I'm glad the issue was raised, but IMO it
>     shouldn't derail progress on an approved blueprint with ready code.
> 
>     Jichen, would you please go ahead and file that blueprint template (no
>     need to write a spec yet) and link it in a review comment on the bottom
>     zvm patch so we have a paper trail?  I'm thinking something like
>     "Consistent platform-specific and optional requirements" -- that leaves
>     us open to decide *how* we're going to "handle" them.
> 
>     Thanks,
>     efried
> 
>     On 04/12/2018 04:13 AM, Chen CH Ji wrote:
>     > Thanks for Michael for raising this question and detailed information
>     > from Clark
>     >
>     > As indicated in the mail, xen, vmware etc might already have this kind
>     > of requirements (and I guess might be more than that) ,
>     > can we accept z/VM requirements first by following other existing ones
>     > then next I can create a BP later to indicate this kind
>     > of change request by referring to Clark's comments and submit patches to
>     > handle it ? Thanks
>     >
>     > Best Regards!
>     >
>     > Kevin (Chen) Ji 纪 晨
>     >
>     > Engineer, zVM Development, CSTL
>     > Notes: Chen CH Ji/China/IBM@IBMCN Internet: jiche...@cn.ibm.com 
> <mailto:jiche...@cn.ibm.com>
>     > Phone: +86-10-82451493
>     > Address: 3/F Ring Building, ZhongGuanCun Software Park, Haidian
>     > District, Beijing 100193, PRC
>     >
>     > Inactive hide details for Matt Riedemann ---04/12/2018 08:46:25 AM---On
>     > 4/11/2018 5:09 PM, Michael Still wrote: >Matt Riedemann ---04/12/2018
>     > 08:46:25 AM---On 4/11/2018 5:09 PM, Michael Still wrote: >
>     >
>     > From: Matt Riedemann <mriede...@gmail.com <mailto:mriede...@gmail.com>>
>     > To: openstack-dev@lists.openstack.org
>     <mailto:openstack-dev@lists.openstack.org>
>     > Date: 04/12/2018 08:46 AM
>     > Subject: Re: [openstack-dev] [Nova][Deployers] Optional, platform
>     > specific, dependancies in requirements.txt
>     >
>     >
>     ------------------------------------------------------------------------
>     >
>     >
>     >
>     > On 4/11/2018 5:09 PM, Michael Still wrote:
>     >>
>     >>
>     >
>     
> https://urldefense.proofpoint.com/v2/url?u=https-3A__review.openstack.org_-23_c_523387&d=DwIGaQ&c=jf_iaSHvJObTbx-siA1ZOg&r=8sI5aZT88Uetyy_XsOddbPjIiLSGM-sFnua3lLy2Xr0&m=212PUwLYOBlJZ3BiZNuJIFkRfqXoBPJDcWYCDk7vCHg&s=CNosrTHnAR21zOI52fnDRfTqu2zPiAn2oW9f67Qijo4&e=
>     
> <https://urldefense.proofpoint.com/v2/url?u=https-3A__review.openstack.org_-23_c_523387&d=DwIGaQ&c=jf_iaSHvJObTbx-siA1ZOg&r=8sI5aZT88Uetyy_XsOddbPjIiLSGM-sFnua3lLy2Xr0&m=212PUwLYOBlJZ3BiZNuJIFkRfqXoBPJDcWYCDk7vCHg&s=CNosrTHnAR21zOI52fnDRfTqu2zPiAn2oW9f67Qijo4&e=>
>  proposes
>     > adding a z/VM specific
>     >> dependancy to nova's requirements.txt. When I objected the counter
>     >> argument is that we have examples of windows specific dependancies
>     >> (os-win) and powervm specific dependancies in that file already.
>     >>
>     >> I think perhaps all three are a mistake and should be removed.
>     >>
>     >> My recollection is that for drivers like ironic which may not be
>     >> deployed by everyone, we have the dependancy documented, and then
>     loaded
>     >> at runtime by the driver itself instead of adding it to
>     >> requirements.txt. This is to stop pip for auto-installing the
>     dependancy
>     >> for anyone who wants to run nova. I had assumed this was at the
>     request
>     >> of the deployer community.
>     >>
>     >> So what do we do with z/VM? Do we clean this up? Or do we now allow
>     >> dependancies that are only useful to a very small number of
>     deployments
>     >> into requirements.txt?
>     >
>     > As Eric pointed out in the review, this came up when pypowervm was
>     added:
>     >
>     >
>     
> https://urldefense.proofpoint.com/v2/url?u=https-3A__review.openstack.org_-23_c_438119_5_requirements.txt&d=DwIGaQ&c=jf_iaSHvJObTbx-siA1ZOg&r=8sI5aZT88Uetyy_XsOddbPjIiLSGM-sFnua3lLy2Xr0&m=212PUwLYOBlJZ3BiZNuJIFkRfqXoBPJDcWYCDk7vCHg&s=iyKxF-CcGAFmnQs8B7d5u2zwEiJqq8ivETmrgB77PEg&e=
>     
> <https://urldefense.proofpoint.com/v2/url?u=https-3A__review.openstack.org_-23_c_438119_5_requirements.txt&d=DwIGaQ&c=jf_iaSHvJObTbx-siA1ZOg&r=8sI5aZT88Uetyy_XsOddbPjIiLSGM-sFnua3lLy2Xr0&m=212PUwLYOBlJZ3BiZNuJIFkRfqXoBPJDcWYCDk7vCHg&s=iyKxF-CcGAFmnQs8B7d5u2zwEiJqq8ivETmrgB77PEg&e=>
>     >
>     > And you're asking the same questions I did in there, which was, should
>     > it go into test-requirements.txt like oslo.vmware and
>     > python-ironicclient, or should it go under [extras], or go into
>     > requirements.txt like os-win (we also have the xenapi library now
>     too).
>     >
>     > I don't really think all of these optional packages should be in
>     > requirements.txt, but we should just be consistent with whatever
>     we do,
>     > be that test-requirements.txt or [extras]. I remember caring more
>     about
>     > this back in my rpm packaging days when we actually tracked what
>     was in
>     > requirements.txt to base what needed to go into the rpm spec, unlike
>     > Fedora rpm specs which just zero out requirements.txt and depend on
>     > their own knowledge of what needs to be installed (which is sometimes
>     > lacking or lagging master).
>     >
>     > I also seem to remember that [extras] was less than user-friendly for
>     > some reason, but maybe that was just because of how our CI jobs are
>     > setup? Or I'm just making that up. I know it's pretty simple to
>     install
>     > the stuff from extras for tox runs, it's just an extra set of
>     > dependencies to list in the tox.ini.
>     >
>     > Having said all this, I don't have the energy to help push for
>     > consistency myself, but will happily watch you from the sidelines.
>     >
>     > --
>     >
>     > Thanks,
>     >
>     > Matt
>     >
>     >
>     __________________________________________________________________________
>     > OpenStack Development Mailing List (not for usage questions)
>     > Unsubscribe:
>     openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
>     <http://openstack-dev-requ...@lists.openstack.org?subject:unsubscribe>
>     >
>     
> https://urldefense.proofpoint.com/v2/url?u=http-3A__lists.openstack.org_cgi-2Dbin_mailman_listinfo_openstack-2Ddev&d=DwIGaQ&c=jf_iaSHvJObTbx-siA1ZOg&r=8sI5aZT88Uetyy_XsOddbPjIiLSGM-sFnua3lLy2Xr0&m=212PUwLYOBlJZ3BiZNuJIFkRfqXoBPJDcWYCDk7vCHg&s=2FioyzCRtztysjjEqCrBTkpQs_wwfs3Mt2wGDkrft-s&e=
>     
> <https://urldefense.proofpoint.com/v2/url?u=http-3A__lists.openstack.org_cgi-2Dbin_mailman_listinfo_openstack-2Ddev&d=DwIGaQ&c=jf_iaSHvJObTbx-siA1ZOg&r=8sI5aZT88Uetyy_XsOddbPjIiLSGM-sFnua3lLy2Xr0&m=212PUwLYOBlJZ3BiZNuJIFkRfqXoBPJDcWYCDk7vCHg&s=2FioyzCRtztysjjEqCrBTkpQs_wwfs3Mt2wGDkrft-s&e=>
>     >
>     >
>     >
>     >
>     >
>     >
>     >
>     __________________________________________________________________________
>     > OpenStack Development Mailing List (not for usage questions)
>     > Unsubscribe:
>     openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
>     <http://openstack-dev-requ...@lists.openstack.org?subject:unsubscribe>
>     > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>     <http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev>
>     >
> 
>     __________________________________________________________________________
>     OpenStack Development Mailing List (not for usage questions)
>     Unsubscribe:
>     openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
>     <http://openstack-dev-requ...@lists.openstack.org?subject:unsubscribe>
>     http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>     <http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev>
> 
> 
> 
> 
> __________________________________________________________________________
> OpenStack Development Mailing List (not for usage questions)
> Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
> 

__________________________________________________________________________
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Reply via email to