We should have a table in Cassandra which stores zip archives associated with a test run.
Created attachment 376270 [details] Patch
Comment on attachment 376270 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=376270&action=review > Tools/resultsdbpy/resultsdbpy/model/archive_context.py:101 > + ttl=int((uuid // Commit.TIMESTAMP_TO_UUID_MULTIPLIER) + self.ttl_seconds - time.time()) if self.ttl_seconds else None, Seems repeated code from initial `ttl` declaration above.
Comment on attachment 376270 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=376270&action=review >> Tools/resultsdbpy/resultsdbpy/model/archive_context.py:101 >> + ttl=int((uuid // Commit.TIMESTAMP_TO_UUID_MULTIPLIER) + self.ttl_seconds - time.time()) if self.ttl_seconds else None, > > Seems repeated code from initial `ttl` declaration above. It would seem like that....I'll change it.
Created attachment 376884 [details] Patch
Comment on attachment 376884 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=376884&action=review rs=me > Tools/resultsdbpy/resultsdbpy/model/archive_context.py:37 > + DEFAULT_LIMIT = 10 DEFAULT_LIMIT for? Can rename it to be more expressive. > Tools/resultsdbpy/resultsdbpy/model/archive_context.py:74 > + self.cassandra.create_table(self.ArchivesByCommit) Just to confirm, we won't be creating a new table each time we instantiate this class, right? > Tools/resultsdbpy/resultsdbpy/model/archive_context.py:89 > + if isinstance(timestamp, datetime): can we use the get_time() method here as well? > Tools/resultsdbpy/resultsdbpy/model/archive_context.py:90 > + timestamp = calendar.timegm(timestamp.timetuple()) why do we use timestamp.timetuple() here but not inside get_time() method? > Tools/resultsdbpy/resultsdbpy/model/archive_context.py:119 > + def get_time(time): you might consider making this stand-alone method(_get_time()), outside find_archive(). Also renaming the parameter 'time' to something like 'input_time' might be more clear. > Tools/resultsdbpy/resultsdbpy/model/archive_context.py:122 > + elif time: should convert this elif to if, since previous if has a corresponding 'return' statement. > Tools/resultsdbpy/resultsdbpy/model/mock_model_factory.py:185 > + old = current - 60 * 60 * 24 * 21 can we store 60 * 60 * 24 * 21 in a variable to make it more readable.
Comment on attachment 376884 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=376884&action=review >> Tools/resultsdbpy/resultsdbpy/model/archive_context.py:74 >> + self.cassandra.create_table(self.ArchivesByCommit) > > Just to confirm, we won't be creating a new table each time we instantiate this class, right? Correct. The CassandraContext class is smart enough to only create the table if it doesn't exist. >> Tools/resultsdbpy/resultsdbpy/model/archive_context.py:90 >> + timestamp = calendar.timegm(timestamp.timetuple()) > > why do we use timestamp.timetuple() here but not inside get_time() method? Because the get_time method is wrong....calendar.timegm(...) requires the tuple
Created attachment 376923 [details] Patch
Comment on attachment 376923 [details] Patch Clearing flags on attachment: 376923 Committed r248979: <https://trac.webkit.org/changeset/248979>
All reviewed patches have been landed. Closing bug.
<rdar://problem/54579632>