Re: [openstack-dev] [Cinder][DRBD] questions about pep8/flake8 etc.

2015-12-22 Thread Gorka Eguileor
On 21/12, Walter A. Boring IV wrote:
> On 12/21/2015 06:40 AM, Philipp Marek wrote:
> >Hi everybody,
> >
> >in the current patch https://review.openstack.org/#/c/259973/1 the test
> >script needs to use a lot of the constant definitions of the backend driver
> >it's using (DRBDmanage).
> >
> >As the DRBDmanage libraries need not be installed on the CI nodes, I'm
> >providing a minimum of upstream files, accumulated in a separate directory
> >- they get imported and "fixed" to the expected location, so that the
> >driver that should be tested runs as if DRBDmanage is installed.
> >
> >
> >My problem is now that the upstream project doesn't accept all the pep8
> >conventions like Openstack does; so the CI run
> > 
> > http://logs.openstack.org/73/259973/1/check/gate-cinder-pep8/5032b16/console.html
> >gives a lot of messages like "E221 multiple spaces before operator" and
> >similar. (It even crashes during AST parsing ;)
> >
> >
> >So, I can see these options now:
> >
> >   * Make pep8 ignore these files - they're only used by one test script,
> > and are never used in production anyway.
> > + Simple
> > + New upstream files can simply be dropped in as needed
> > - bad example?
> >   * Reformat the files to conform to pep8
> > - some work for every new version that needs to be incorporated
> > - can't be compared for equality with upstream any more
> > - might result in mismatches later on, ie. production code uses
> >   different values from test code
> I would suggest this option.  We don't want to allow code in cinder that
> bypasses the pep8 checks.  Since you are trying to get drbd support into
> Cinder, it then falls upon you to make sure the code you are submitting
> follows the same standards as the rest of the project.
> 
> Walt
> 

I would normally suggest option #4, but considering listed downsides
this looks like the best alternative, as Walter suggests.

To reduce the work for every release you could probably automate it
using autopep8.

Cheers,
Gorka.

> >
> >   * Throw upstream files away, and do "manual" fakes
> > - A lot of work
> > - Work needed for every new needed constant
> > - lots of duplicated code
> > - might result in mismatches later on, ie. production code uses
> >   different values from test code
> > + whole checkout still "clean" for pep8
> >
> >   * Require DRBDmanage to be installed
> > + uses same values as upstream and production
> > - Need to get it upstream into PyPi
> > - Meaning delay
> > - delay for every new release of DRBDmanage
> > - Might not even be compatible with every used distribution/CI
> >   out there
> >
> >
> >I would prefer the first option - make pep8 ignore these files.
> >But I'm only a small player here, what's the opinion of the Cinder cores?
> >Would that be acceptable?
> >
> >
> >Regards,
> >
> >Phil
> >
> >__
> >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

__
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


Re: [openstack-dev] [Cinder][DRBD] questions about pep8/flake8 etc.

2015-12-21 Thread Walter A. Boring IV

On 12/21/2015 06:40 AM, Philipp Marek wrote:

Hi everybody,

in the current patch https://review.openstack.org/#/c/259973/1 the test
script needs to use a lot of the constant definitions of the backend driver
it's using (DRBDmanage).

As the DRBDmanage libraries need not be installed on the CI nodes, I'm
providing a minimum of upstream files, accumulated in a separate directory
- they get imported and "fixed" to the expected location, so that the
driver that should be tested runs as if DRBDmanage is installed.


My problem is now that the upstream project doesn't accept all the pep8
conventions like Openstack does; so the CI run
 
http://logs.openstack.org/73/259973/1/check/gate-cinder-pep8/5032b16/console.html
gives a lot of messages like "E221 multiple spaces before operator" and
similar. (It even crashes during AST parsing ;)


So, I can see these options now:

   * Make pep8 ignore these files - they're only used by one test script,
 and are never used in production anyway.
 + Simple
 + New upstream files can simply be dropped in as needed
 - bad example?
   
   * Reformat the files to conform to pep8

 - some work for every new version that needs to be incorporated
 - can't be compared for equality with upstream any more
 - might result in mismatches later on, ie. production code uses
   different values from test code
I would suggest this option.  We don't want to allow code in cinder that 
bypasses the pep8 checks.  Since you are trying to get drbd support into 
Cinder, it then falls upon you to make sure the code you are submitting 
follows the same standards as the rest of the project.


Walt



   * Throw upstream files away, and do "manual" fakes
 - A lot of work
 - Work needed for every new needed constant
 - lots of duplicated code
 - might result in mismatches later on, ie. production code uses
   different values from test code
 + whole checkout still "clean" for pep8

   * Require DRBDmanage to be installed
 + uses same values as upstream and production
 - Need to get it upstream into PyPi
 - Meaning delay
 - delay for every new release of DRBDmanage
 - Might not even be compatible with every used distribution/CI
   out there


I would prefer the first option - make pep8 ignore these files.
But I'm only a small player here, what's the opinion of the Cinder cores?
Would that be acceptable?


Regards,

Phil

__
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


[openstack-dev] [Cinder][DRBD] questions about pep8/flake8 etc.

2015-12-21 Thread Philipp Marek
Hi everybody,

in the current patch https://review.openstack.org/#/c/259973/1 the test 
script needs to use a lot of the constant definitions of the backend driver 
it's using (DRBDmanage).

As the DRBDmanage libraries need not be installed on the CI nodes, I'm 
providing a minimum of upstream files, accumulated in a separate directory 
- they get imported and "fixed" to the expected location, so that the 
driver that should be tested runs as if DRBDmanage is installed.


My problem is now that the upstream project doesn't accept all the pep8 
conventions like Openstack does; so the CI run 

http://logs.openstack.org/73/259973/1/check/gate-cinder-pep8/5032b16/console.html
 
gives a lot of messages like "E221 multiple spaces before operator" and 
similar. (It even crashes during AST parsing ;)


So, I can see these options now:

  * Make pep8 ignore these files - they're only used by one test script,
and are never used in production anyway.
+ Simple
+ New upstream files can simply be dropped in as needed
- bad example?
  
  * Reformat the files to conform to pep8
- some work for every new version that needs to be incorporated
- can't be compared for equality with upstream any more
- might result in mismatches later on, ie. production code uses
  different values from test code

  * Throw upstream files away, and do "manual" fakes
- A lot of work
- Work needed for every new needed constant
- lots of duplicated code
- might result in mismatches later on, ie. production code uses
  different values from test code
+ whole checkout still "clean" for pep8

  * Require DRBDmanage to be installed
+ uses same values as upstream and production
- Need to get it upstream into PyPi
- Meaning delay
- delay for every new release of DRBDmanage
- Might not even be compatible with every used distribution/CI
  out there


I would prefer the first option - make pep8 ignore these files.
But I'm only a small player here, what's the opinion of the Cinder cores?
Would that be acceptable?


Regards,

Phil

__
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