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)
     
     
    Show all issues
    Col: 5
     E265 block comment should start with '# '
    
  3. reviewboard/accounts/models.py (Diff revision 1)
     
     
    Show all issues
    Col: 43
     E251 unexpected spaces around keyword / parameter equals
    
  4. reviewboard/accounts/models.py (Diff revision 1)
     
     
    Show all issues
    Col: 45
     E251 unexpected spaces around keyword / parameter equals
    
  5. reviewboard/accounts/models.py (Diff revision 1)
     
     
    Show all issues
    Col: 56
     E251 unexpected spaces around keyword / parameter equals
    
  6. reviewboard/accounts/models.py (Diff revision 1)
     
     
    Show all issues
    Col: 58
     E251 unexpected spaces around keyword / parameter equals
    
  7. reviewboard/accounts/models.py (Diff revision 1)
     
     
    Show all issues
    Col: 5
     E265 block comment should start with '# '
    
  8. reviewboard/accounts/models.py (Diff revision 1)
     
     
    Show all issues
    Col: 56
     W291 trailing whitespace
    
  9. reviewboard/accounts/models.py (Diff revision 1)
     
     
    Show all issues
    Col: 48
     E251 unexpected spaces around keyword / parameter equals
    
  10. reviewboard/accounts/models.py (Diff revision 1)
     
     
    Show all issues
    Col: 50
     E251 unexpected spaces around keyword / parameter equals
    
  11. reviewboard/accounts/models.py (Diff revision 1)
     
     
    Show all issues
    Col: 61
     E251 unexpected spaces around keyword / parameter equals
    
  12. reviewboard/accounts/models.py (Diff revision 1)
     
     
    Show all issues
    Col: 63
     E251 unexpected spaces around keyword / parameter equals
    
  13. reviewboard/accounts/models.py (Diff revision 1)
     
     
    Show all issues
     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. Show all issues
    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. Show all issues

    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. Show all issues
    Col: 80
     E501 line too long (96 > 79 characters)
    
  3. reviewboard/attachments/mimetypes.py (Diff revision 4)
     
     
    Show all issues
    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. Show all issues
    Col: 80
     E501 line too long (96 > 79 characters)
    
  3. reviewboard/attachments/mimetypes.py (Diff revision 5)
     
     
    Show all issues
    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. Show all issues

    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)
     
     
    Show all issues

    Imports should be alphabetical.

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

    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)
     
     
     
    Show all issues

    Remove this blank line.

  6. reviewboard/attachments/forms.py (Diff revision 6)
     
     
    Show all issues

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

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

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

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

    Alphabetical.

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

    Alphabetical.

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

    Should name this 'file_attachments'.

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

    No blank line here.

  12. Show all issues

    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)
     
     
    Show all issues
     'guess_mimetype' imported but unused
    
  3. reviewboard/attachments/forms.py (Diff revision 8)
     
     
    Show all issues
     undefined name 'is_exe_in_path'
    
  4. reviewboard/attachments/forms.py (Diff revision 8)
     
     
    Show all issues
     undefined name 'subprocess'
    
  5. reviewboard/attachments/forms.py (Diff revision 8)
     
     
    Show all issues
     undefined name 'subprocess'
    
  6. reviewboard/attachments/forms.py (Diff revision 8)
     
     
    Show all issues
     undefined name 'subprocess'
    
  7. reviewboard/attachments/forms.py (Diff revision 8)
     
     
    Show all issues
     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)
     
     
    Show all issues
     'guess_mimetype' imported but unused
    
  3. reviewboard/attachments/forms.py (Diff revision 9)
     
     
    Show all issues
     undefined name 'is_exe_in_path'
    
  4. reviewboard/attachments/forms.py (Diff revision 9)
     
     
    Show all issues
     undefined name 'subprocess'
    
  5. reviewboard/attachments/forms.py (Diff revision 9)
     
     
    Show all issues
     undefined name 'subprocess'
    
  6. reviewboard/attachments/forms.py (Diff revision 9)
     
     
    Show all issues
     undefined name 'subprocess'
    
  7. reviewboard/attachments/forms.py (Diff revision 9)
     
     
    Show all issues
     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)
     
     
     
     
     
    Show all issues

    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:
Completed