[ditrack commit] r2368 - src/trunk/DITrack/DB

[Vlad Skvortsov]

Oleg,

please create a separate branch (and include that in autotest) for the 
follow-up changes, as agreed.

My comments inline.


oleg at ditrack.org wrote:
> Author: oleg
> Date: 2007-11-18 09:09:52 -0800 (Sun, 18 Nov 2007)
> New Revision: 2368
>
> Added:
>    src/trunk/DITrack/DB/Cache.py
> Log:
> added simple cache interface
>
>
> Added: src/trunk/DITrack/DB/Cache.py
> ===================================================================
> --- src/trunk/DITrack/DB/Cache.py	                        (rev 0)
> +++ src/trunk/DITrack/DB/Cache.py	2007-11-18 17:09:52 UTC (rev 2368)
> @@ -0,0 +1,75 @@
> +#
> +# Cache.py - cache interface
> +#
> +# Copyright (c) 2006-2007 The DITrack Project, www.ditrack.org.
>   

...2008.

> +#
> +# $Id$
> +# $HeadURL$
> +#
> +# Redistribution and use in source and binary forms, with or without
> +# modification, are permitted provided that the following conditions are met:
> +#
> +#  * Redistributions of source code must retain the above copyright notice,
> +# this list of conditions and the following disclaimer.
> +#  * Redistributions in binary form must reproduce the above copyright notice,
> +# this list of conditions and the following disclaimer in the documentation
> +# and/or other materials provided with the distribution.
> +#
> +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
> +# AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> +# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
> +# ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE
> +# LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
> +# CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
> +# SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
> +# INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
> +# CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
> +# ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
> +# POSSIBILITY OF SUCH DAMAGE.
> +#
> +
> +import time
> +
> +# DITrack modules
> +import DITrack.DB.Common
> +
> +CACHE_EXPIRED_TIME = 6 # in hours (numeric type only)
>   

Why do we need time-based expiration? I think we should rely on actual 
changes (read: commits to the database tree) and the time passed since 
we cached data doesn't make any difference for us.

> +CACHE_FILE = "cache"
> +CACHE_VERSION = "0.1"
>   

Why not just "1"?


> +
> +#
> +# Classes
> +#
> +
> +class Cache:
> +    """
> +    Class encapsulating cache operations.
> +    """
>   

...cache of what?


> +
> +    def __del__(self):
> +        self.cache.close()
>   

My recent experience with closing shelves from destructors suggests that 
this kind of finalization should be better avoided. Sometimes the 
objects are still mysteriously referenced from somewhere else and the 
data doesn't get flushed to the disk. I'm not sure whether we should add 
an explicit method to close the shelve here.

> +
> +    def __init__(self, path):
> +        """
> +        PATH is a path to an issue database
> +        """
>   

Missing trailing period.

> +        self.cache = DITrack.DB.Common.open_shelving(path, CACHE_FILE)
> +
> +    def __len__(self):
>   

Comment?

> +        return len(self.get())
> +
> +    def clear(self):
>   

Comment?

> +        if len(self):
> +            self.cache["data"] = []
> +
> +    def get(self):
>   

Comment?

> +        if ("data" in self.cache and self.cache["expired_time"] > time.time()
> +            and self.cache["version"] == CACHE_VERSION):
> +                return self.cache["data"]
> +        return []
> +
> +    def set(self, issues):
>   

Comment?
> +        if len(issues):
> +            self.cache["data"] = issues
> +            self.cache["expired_time"] = time.time() + CACHE_EXPIRED_TIME * 3600
> +            self.cache["version"] = CACHE_VERSION
>
>   

-- 
Vlad Skvortsov, vss at 73rus.com, http://vss.73rus.com