Per-user file storage model changes

Review Request #6022 — Created June 23, 2014 and submitted

Information

Review Board
master
1f7043c...

Reviewers

Model changes for enabling ownership of FileAttachments

  • Evolutions manually run successfully
  • Test suite run
Description From Last Updated

Col: 5 E265 block comment should start with '# '

reviewbotreviewbot

Col: 43 E251 unexpected spaces around keyword / parameter equals

reviewbotreviewbot

Col: 45 E251 unexpected spaces around keyword / parameter equals

reviewbotreviewbot

Col: 56 E251 unexpected spaces around keyword / parameter equals

reviewbotreviewbot

Col: 58 E251 unexpected spaces around keyword / parameter equals

reviewbotreviewbot

Col: 5 E265 block comment should start with '# '

reviewbotreviewbot

Col: 56 W291 trailing whitespace

reviewbotreviewbot

Col: 48 E251 unexpected spaces around keyword / parameter equals

reviewbotreviewbot

Col: 50 E251 unexpected spaces around keyword / parameter equals

reviewbotreviewbot

Col: 61 E251 unexpected spaces around keyword / parameter equals

reviewbotreviewbot

Col: 63 E251 unexpected spaces around keyword / parameter equals

reviewbotreviewbot

undefined name 'self'

reviewbotreviewbot

Col: 80 E501 line too long (95 > 79 characters)

reviewbotreviewbot

The first line of this file should be from __future__ import unicode_literals

daviddavid

Col: 80 E501 line too long (96 > 79 characters)

reviewbotreviewbot

Col: 1 E302 expected 2 blank lines, found 1

reviewbotreviewbot

Col: 80 E501 line too long (96 > 79 characters)

reviewbotreviewbot

Col: 1 E302 expected 2 blank lines, found 1

reviewbotreviewbot

You'll need to prepend this line to the file, followed by a blank line: from __future__ import unicode_literals This ensures …

chipx86chipx86

Imports should be alphabetical.

chipx86chipx86

Since file and review_request are now optional, they should default to None.

chipx86chipx86

Remove this blank line.

chipx86chipx86

This is referencing file, since it's None. Doesn't look like this form was tested?

chipx86chipx86

You need 2 blank lines between classes. You removed one of them.

chipx86chipx86

Alphabetical.

chipx86chipx86

Alphabetical.

chipx86chipx86

Should name this 'file_attachments'.

chipx86chipx86

No blank line here.

chipx86chipx86

Since we're dealing with optional keyword arguments now, can you do: form.create(file=..., review_request=...)

chipx86chipx86

'guess_mimetype' imported but unused

reviewbotreviewbot

undefined name 'is_exe_in_path'

reviewbotreviewbot

undefined name 'subprocess'

reviewbotreviewbot

undefined name 'subprocess'

reviewbotreviewbot

undefined name 'subprocess'

reviewbotreviewbot

undefined name 'subprocess'

reviewbotreviewbot

'guess_mimetype' imported but unused

reviewbotreviewbot

undefined name 'is_exe_in_path'

reviewbotreviewbot

undefined name 'subprocess'

reviewbotreviewbot

undefined name 'subprocess'

reviewbotreviewbot

undefined name 'subprocess'

reviewbotreviewbot

undefined name 'subprocess'

reviewbotreviewbot

Too many blank lines. Should only be one at this level.

chipx86chipx86
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/accounts/models.py
        reviewboard/attachments/models.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/accounts/models.py
        reviewboard/attachments/models.py
    
    
  2. reviewboard/accounts/models.py (Diff revision 1)
     
     
    Col: 5
     E265 block comment should start with '# '
    
  3. reviewboard/accounts/models.py (Diff revision 1)
     
     
    Col: 43
     E251 unexpected spaces around keyword / parameter equals
    
  4. reviewboard/accounts/models.py (Diff revision 1)
     
     
    Col: 45
     E251 unexpected spaces around keyword / parameter equals
    
  5. reviewboard/accounts/models.py (Diff revision 1)
     
     
    Col: 56
     E251 unexpected spaces around keyword / parameter equals
    
  6. reviewboard/accounts/models.py (Diff revision 1)
     
     
    Col: 58
     E251 unexpected spaces around keyword / parameter equals
    
  7. reviewboard/accounts/models.py (Diff revision 1)
     
     
    Col: 5
     E265 block comment should start with '# '
    
  8. reviewboard/accounts/models.py (Diff revision 1)
     
     
    Col: 56
     W291 trailing whitespace
    
  9. reviewboard/accounts/models.py (Diff revision 1)
     
     
    Col: 48
     E251 unexpected spaces around keyword / parameter equals
    
  10. reviewboard/accounts/models.py (Diff revision 1)
     
     
    Col: 50
     E251 unexpected spaces around keyword / parameter equals
    
  11. reviewboard/accounts/models.py (Diff revision 1)
     
     
    Col: 61
     E251 unexpected spaces around keyword / parameter equals
    
  12. reviewboard/accounts/models.py (Diff revision 1)
     
     
    Col: 63
     E251 unexpected spaces around keyword / parameter equals
    
  13. reviewboard/accounts/models.py (Diff revision 1)
     
     
     undefined name 'self'
    
  14. 
      
RV
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/attachments/tests.py
        reviewboard/attachments/evolutions/__init__.py
        reviewboard/attachments/models.py
        reviewboard/attachments/forms.py
        reviewboard/attachments/evolutions/file_attachment_ownership.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/attachments/tests.py
        reviewboard/attachments/evolutions/__init__.py
        reviewboard/attachments/models.py
        reviewboard/attachments/forms.py
        reviewboard/attachments/evolutions/file_attachment_ownership.py
    
    
  2. Col: 80
     E501 line too long (95 > 79 characters)
    
  3. 
      
RV
reviewbot
  1. Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/attachments/tests.py
        reviewboard/attachments/evolutions/__init__.py
        reviewboard/attachments/models.py
        reviewboard/attachments/forms.py
        reviewboard/attachments/evolutions/file_attachment_ownership.py
    
    
    
    Tool: Pyflakes
    Processed Files:
        reviewboard/attachments/tests.py
        reviewboard/attachments/evolutions/__init__.py
        reviewboard/attachments/models.py
        reviewboard/attachments/forms.py
        reviewboard/attachments/evolutions/file_attachment_ownership.py
    
    
  2. 
      
david
  1. 
      
  2. The first line of this file should be from __future__ import unicode_literals

  3. 
      
RV
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/attachments/evolutions/__init__.py
        reviewboard/attachments/mimetypes.py
        reviewboard/attachments/models.py
        reviewboard/attachments/forms.py
        reviewboard/attachments/evolutions/file_attachment_ownership.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/attachments/evolutions/__init__.py
        reviewboard/attachments/mimetypes.py
        reviewboard/attachments/models.py
        reviewboard/attachments/forms.py
        reviewboard/attachments/evolutions/file_attachment_ownership.py
    
    
  2. Col: 80
     E501 line too long (96 > 79 characters)
    
  3. reviewboard/attachments/mimetypes.py (Diff revision 4)
     
     
    Col: 1
     E302 expected 2 blank lines, found 1
    
  4. 
      
RV
RV
reviewbot
  1. Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/webapi/resources/base_file_attachment.py
        reviewboard/attachments/models.py
        reviewboard/attachments/evolutions/__init__.py
        reviewboard/attachments/mimetypes.py
        reviewboard/attachments/evolutions/file_attachment_ownership.py
        reviewboard/attachments/forms.py
    
    
    
    Tool: Pyflakes
    Processed Files:
        reviewboard/webapi/resources/base_file_attachment.py
        reviewboard/attachments/models.py
        reviewboard/attachments/evolutions/__init__.py
        reviewboard/attachments/mimetypes.py
        reviewboard/attachments/evolutions/file_attachment_ownership.py
        reviewboard/attachments/forms.py
    
    
  2. Col: 80
     E501 line too long (96 > 79 characters)
    
  3. reviewboard/attachments/mimetypes.py (Diff revision 5)
     
     
    Col: 1
     E302 expected 2 blank lines, found 1
    
  4. 
      
RV
reviewbot
  1. Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/webapi/resources/base_file_attachment.py
        reviewboard/attachments/models.py
        reviewboard/attachments/evolutions/__init__.py
        reviewboard/attachments/mimetypes.py
        reviewboard/attachments/evolutions/file_attachment_ownership.py
        reviewboard/attachments/forms.py
    
    
    
    Tool: Pyflakes
    Processed Files:
        reviewboard/webapi/resources/base_file_attachment.py
        reviewboard/attachments/models.py
        reviewboard/attachments/evolutions/__init__.py
        reviewboard/attachments/mimetypes.py
        reviewboard/attachments/evolutions/file_attachment_ownership.py
        reviewboard/attachments/forms.py
    
    
  2. 
      
chipx86
  1. I thought we decided that UploadFileForm was going to be refactored to have a base class for the common bits, and then subclasses for general file uploading an review request-related uploading?

    1. I decided to simplify my create method so it always creates an empty FileAttachment associated with the user that made the request. I moved guess_mimetype because I intend on performing the check when the file is ultimately provided in the PUT request (directly). I have realized that in this case my changes to UploadFileForm.create are unncessary since my create method does not use it to construct the FileAttachment (it ignores all POST parameters as mentioned).

  2. You'll need to prepend this line to the file, followed by a blank line:

    from __future__ import unicode_literals
    

    This ensures all strings in our codebase are in Unicode format.

  3. reviewboard/attachments/forms.py (Diff revision 6)
     
     

    Imports should be alphabetical.

  4. reviewboard/attachments/forms.py (Diff revision 6)
     
     

    Since file and review_request are now optional, they should default to None.

    1. I decided to preserve UploadFileForm.create since my resource will not use it.

  5. reviewboard/attachments/forms.py (Diff revision 6)
     
     
     

    Remove this blank line.

  6. reviewboard/attachments/forms.py (Diff revision 6)
     
     

    This is referencing file, since it's None. Doesn't look like this form was tested?

  7. reviewboard/attachments/forms.py (Diff revision 6)
     
     

    You need 2 blank lines between classes. You removed one of them.

  8. reviewboard/attachments/mimetypes.py (Diff revision 6)
     
     

    Alphabetical.

  9. reviewboard/attachments/models.py (Diff revision 6)
     
     

    Alphabetical.

  10. reviewboard/attachments/models.py (Diff revision 6)
     
     

    Should name this 'file_attachments'.

  11. reviewboard/attachments/models.py (Diff revision 6)
     
     
     
     

    No blank line here.

  12. Since we're dealing with optional keyword arguments now, can you do:

    form.create(file=..., review_request=...)
    
    1. Given that I have decided to preserve UploadFileForm.create I have made path mandatory in base_file_attachment. Since my resource will operate independently and this can preserve the existing DnD functionality for review-request associated FileAttachments.

  13. 
      
RV
reviewbot
  1. Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/webapi/resources/base_file_attachment.py
        reviewboard/attachments/models.py
        reviewboard/attachments/evolutions/__init__.py
        reviewboard/attachments/mimetypes.py
        reviewboard/attachments/evolutions/file_attachment_ownership.py
        reviewboard/attachments/forms.py
    
    
  2. 
      
RV
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/attachments/evolutions/__init__.py
        reviewboard/attachments/mimetypes.py
        reviewboard/attachments/models.py
        reviewboard/attachments/forms.py
        reviewboard/attachments/evolutions/file_attachment_ownership.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/attachments/evolutions/__init__.py
        reviewboard/attachments/mimetypes.py
        reviewboard/attachments/models.py
        reviewboard/attachments/forms.py
        reviewboard/attachments/evolutions/file_attachment_ownership.py
    
    
  2. reviewboard/attachments/forms.py (Diff revision 8)
     
     
     'guess_mimetype' imported but unused
    
  3. reviewboard/attachments/forms.py (Diff revision 8)
     
     
     undefined name 'is_exe_in_path'
    
  4. reviewboard/attachments/forms.py (Diff revision 8)
     
     
     undefined name 'subprocess'
    
  5. reviewboard/attachments/forms.py (Diff revision 8)
     
     
     undefined name 'subprocess'
    
  6. reviewboard/attachments/forms.py (Diff revision 8)
     
     
     undefined name 'subprocess'
    
  7. reviewboard/attachments/forms.py (Diff revision 8)
     
     
     undefined name 'subprocess'
    
  8. 
      
RV
reviewbot
  1. Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/attachments/evolutions/__init__.py
        reviewboard/attachments/mimetypes.py
        reviewboard/attachments/models.py
        reviewboard/attachments/forms.py
        reviewboard/attachments/evolutions/file_attachment_ownership.py
    
    
    
    Tool: Pyflakes
    Processed Files:
        reviewboard/attachments/evolutions/__init__.py
        reviewboard/attachments/mimetypes.py
        reviewboard/attachments/models.py
        reviewboard/attachments/forms.py
        reviewboard/attachments/evolutions/file_attachment_ownership.py
    
    
  2. reviewboard/attachments/forms.py (Diff revision 9)
     
     
     'guess_mimetype' imported but unused
    
  3. reviewboard/attachments/forms.py (Diff revision 9)
     
     
     undefined name 'is_exe_in_path'
    
  4. reviewboard/attachments/forms.py (Diff revision 9)
     
     
     undefined name 'subprocess'
    
  5. reviewboard/attachments/forms.py (Diff revision 9)
     
     
     undefined name 'subprocess'
    
  6. reviewboard/attachments/forms.py (Diff revision 9)
     
     
     undefined name 'subprocess'
    
  7. reviewboard/attachments/forms.py (Diff revision 9)
     
     
     undefined name 'subprocess'
    
  8. 
      
RV
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/attachments/evolutions/__init__.py
        reviewboard/attachments/mimetypes.py
        reviewboard/attachments/models.py
        reviewboard/attachments/forms.py
        reviewboard/attachments/evolutions/file_attachment_ownership.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/attachments/evolutions/__init__.py
        reviewboard/attachments/mimetypes.py
        reviewboard/attachments/models.py
        reviewboard/attachments/forms.py
        reviewboard/attachments/evolutions/file_attachment_ownership.py
    
    
  2. 
      
chipx86
  1. This looks good! One small thing from me. I want to make sure everyone else is happy too, but then this should be able to go in.

  2. reviewboard/attachments/forms.py (Diff revision 10)
     
     
     
     
     

    Too many blank lines. Should only be one at this level.

  3. 
      
RV
reviewbot
  1. Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/attachments/evolutions/__init__.py
        reviewboard/attachments/mimetypes.py
        reviewboard/attachments/models.py
        reviewboard/attachments/forms.py
        reviewboard/attachments/evolutions/file_attachment_ownership.py
    
    
    
    Tool: Pyflakes
    Processed Files:
        reviewboard/attachments/evolutions/__init__.py
        reviewboard/attachments/mimetypes.py
        reviewboard/attachments/models.py
        reviewboard/attachments/forms.py
        reviewboard/attachments/evolutions/file_attachment_ownership.py
    
    
  2. 
      
RV
  1. Ship It!

  2. 
      
RV
Review request changed

Status: Closed (submitted)

Loading...