Differential D6424 Diff 23469 swh/perfecthash/__init__.py

Changeset View

Standalone View

View Options

swh/perfecthash/init.py

from _hash_cffi import lib

ardumontUnsubmitted

Done

+ # See the AUTHORS file at the top-level directory of this distribution

+ # License: GNU General Public License version 3, or any later version

+ # See top-level LICENSE file for more information

from _hash_cffi import lib

from cffi import FFI

ardumont: copyright headers to match other swh modules?

dacharyAuthorUnsubmitted

Done

Good point, done!

dachary: Good point, done!

class Shard:

def __init__(self, path):

self.shard = lib.shard_init(path.encode('utf-8'))

def __del__(self):

lib.shard_destroy(self.shard)

def create(self):

lib.shard_create(self.shard)

return self

def load(self):

lib.shard_load(self.shard)

return self

ardumontUnsubmitted

Done

and looking up the content of an object when given the key.

"""

- def __init__(self, path):

+ def __init__(self, path: str):

"""Initialize with an existing Read Shard.

Args:

- path (str): path to an existing Read Shard file or device

+ path: path to an existing Read Shard file or device

"""

self.ffi = FFI()

Let's add some types.

ardumont: Let's add some types.

def save(self):

lib.shard_save(self.shard)

def lookup(self, key):

object_pointer = ffi.new("char**")

object_size_pointer = ffi.new("size_t*")

lib.shard_lookup(self.shard, key, object, object_size)

return ffi.buffer(object_pointer[0], object_size_pointer[0])

def write(self, key, object):

lib.shard_object_write(self.shard, key, object, len(object))

ardumontUnsubmitted

Done

lib.shard_destroy(self.shard)

- def create(self, objects_count):

+ def create(self, objects_count: int) -> "Shard":

"""Wipe out the content of the Read Shard. It must be followed by

**object_count** calls to the **write** method otherwise the content

of the Read Shard will be inconsistent. When all objects are inserted,

the Read Shard must be made persistent by calling the **save** method.

Args:

- objects_count (int): number of objects in the Read Shard.

+ objects_count: number of objects in the Read Shard.

Returns:

fyi, we now are documenting directly the types within the code and no longer in the docstring.

ardumont: fyi, we now are documenting directly the types within the code and no longer in the docstring.

ardumontUnsubmitted

Done

indent is off by 2 chars almost everywhere.
We normalize on a 4 chars indent below "Args:" and "Returns:" (and "Raises:" if any).

ardumont: indent is off by 2 chars almost everywhere. We normalize on a 4 chars indent below "Args:" and…

ardumontUnsubmitted

Done

return lib.shard_save(self.shard)

- def lookup(self, key):

+ def lookup(self, key: Key) -> HashObject:

"""Fetch the object matching the key in the Read Shard.

Fetching an object is O(1): one lookup in the index to obtain

the offset of the object in the Read Shard and one read to get

the payload.

Args:

- key (32 bytes): the key associated with the object to retrieve.

+ key: the key associated with the object to retrieve.

Returns:

- the object as bytes.

+ the object as bytes.

"""

object_pointer = self.ffi.new("char**")

Let's add some types around this, then define at the top-level module something like:

from typing import NewType

Key = NewType("Key", bytes)
HashObjects = NewType("HashObject", bytes)

I believe that improves the reading significantly.

ardumont: Let's add some types around this, then define at the top-level module something like: ``` from…

ardumontUnsubmitted

Done

return self.ffi.buffer(object_pointer[0], object_size_pointer[0])

- def write(self, key, object):

+ def write(self, key: Key, object: HashObject) -> int:

"""Add the key/object pair to the Read Shard.

The **create** method must have been called prior to calling

the **write** method.

Args:

- key (32 bytes): the unique key associated with the object.

- object (bytes): the object

+ key: the unique key associated with the object.

+ object: the object

Returns:

- 0 on success, -1 on error.

+ 0 on success, -1 on error.

"""

return lib.shard_object_write(self.shard, key, object, len(object))

ardumont:

ardumontUnsubmitted

Done

Something hit me, can't the lookup fail?

What would be the return if that was the case None?
(if so, the type must be changed to Optional[HashObject]).

ardumont: Something hit me, can't the lookup fail? What would be the return if that was the case None?

dacharyAuthorUnsubmitted

Done

I overlooked this question, sorry. The lookup cannot fail: if given a key that does not exist it will return a random object. Which is ok because the global index ensures the Read Shard is only queried with objects that are known to exist in the hash map.

dachary: I overlooked this question, sorry. The lookup cannot fail: if given a key that does not exist…

ardumontUnsubmitted

Done

you did not overlook it, it's a new one!

If anything, that's me who overlooked that code last time ;)

Great! "Can't happen!"

ardumont: you did not overlook it, it's a new one! If anything, that's me who overlooked that code last…

ardumontUnsubmitted

Done

return self

- def save(self):

+ def save(self) -> int:

"""Create the perfect hash table the **lookup** method

From the docstring ;)

ardumont: From the docstring ;)

ardumontUnsubmitted

Done

return self

- def load(self):

+ def load(self) -> "Shard":

"""Open the Read Shard file in read-only mode.

From the docstring.

ardumont: From the docstring.