← Back to team overview

launchpad-dev team mailing list archive

Re: [Branch ~launchpad-pqm/launchpad/devel] Rev 10012: [r=mwhudson][ui=none] Allow listing sftp directories on

 

Bjorn Tillenius wrote:
>> +            for attribute in ['st_uid', 'st_gid', 'st_mtime', 'st_nlink']:
>> +                if getattr(stat_result, attribute, None) is None:
>> +                    setattr(stat_result, attribute, 0)
> 
> This code is very unobvious to me. Why do you have to replace None with
> 0?

I don't.  None is not a valid value for any of the named attributes, so
it's a useful sentry.  If getattr(stat_result, attribute, None) returns
None, the attribute did not exist at all.

When the input is a MemoryStat, I have to add these attributes because
lsLine requires them to be set.


>> +    def test__format_directory_entries_with_MemoryStat(self):
>> +        """format_directory_entries works with MemoryStat.
>> +
>> +        MemoryStat lacks many fields, but format_directory_entries works
>> +        around that.
>> +        """
>> +        t = MemoryTransport()
>> +        stat_result = t.stat('.')
>> +        entries = self.sftp_server._format_directory_entries(
>> +            [stat_result], ['filename'])
>> +        self.assertEqual(list(entries), [
>> +            ('filename', 'drwxr-xr-x    0 0        0               0 '
>> +             'Jan 01  1970 filename',
>> +             {'atime': 0,
>> +              'gid': 0,
>> +              'mtime': 0,
>> +              'permissions': 16877,
>> +              'size': 0,
>> +              'uid': 0})])
>> +        self.assertIs(None, getattr(stat_result, 'st_mtime', None))
> 
> What's this last assert for? Why don't you care whether stat_result has
> an st_mtime attribute?

I do care, very much.  I want to to *not* have an st_mtime attribute,
because it didn't have one before it was passed into
_format_directory_entries, and _format_directory_entries should not
modify its input.

Aaron



Follow ups

References