Export the built tree as git repo #9

Merged
Ghost merged 24 commits from add_export into main 2022-11-02 14:17:24 +01:00
Showing only changes of commit a1ead29734 - Show all commits

View File

@ -1,5 +1,8 @@
from __future__ import annotations
import logging
from hashlib import md5
from typing import Optional
from lib.db import DB
from lib.request import Request
@ -204,18 +207,24 @@ class DBRevision:
self._files.sort(key=lambda x: x["name"])
Ghost marked this conversation as resolved Outdated

Reading the code seems that current_rev is not optional.

Reading the code seems that `current_rev` is not optional.
Outdated
Review

There is a if current_rev - and it's treated as empty dict if there is nothing before.

There is a `if current_rev` - and it's treated as empty dict if there is nothing before.

but not in the call?

but not in the call?
Outdated
Review

well, the =None is not needed as there is always a variable passed. I guess nowadays I can make it current_rev: Optional[DbRevision]. Would that be more of your liking?

well, the `=None` is not needed as there is always a variable passed. I guess nowadays I can make it `current_rev: Optional[DbRevision]`. Would that be more of your liking?

What I am wondering is the semantic of the function. calc_delta seems to be from some status to self. When the other status is not None the old_files is empty making all to download, when I would expect that none would be listed to download.

That makes me wonder if current_rev can be optional.

What I am wondering is the semantic of the function. `calc_delta` seems to be from some status to `self`. When the other status is not `None` the `old_files` is empty making all to download, when I would expect that none would be listed to download. That makes me wonder if `current_rev` can be optional.
Outdated
Review

I even added documentation! :)

self is the revision to download and current_rev is the state of the repository. If it's None, there is no state and everything needs to be downloaded.

I even added documentation! :) `self` is the revision to download and `current_rev` is the state of the repository. If it's `None`, there is no state and everything needs to be downloaded.
return self._files
def calc_delta(self, db: DB, current_rev=None):
"""Calculate the list of files to download and to delete"""
old_files = dict()
def calc_delta(self, db: DB, current_rev: Optional[DBRevision]):
"""Calculate the list of files to download and to delete.
Param current_rev is the revision that's currently checked out.
Ghost marked this conversation as resolved Outdated

(no optional)

(no optional)
If it's None, the repository is empty.
"""
Ghost marked this conversation as resolved Outdated

The last two lines: old_files = {e["name"]: f"{e['md5']}-{e['size']}" for e in current_rev.files_list(db)}

The last two lines: `old_files = {e["name"]: f"{e['md5']}-{e['size']}" for e in current_rev.files_list(db)}`
Outdated
Review

But current_rev is optional - no matter what you make out of it :)

But current_rev is optional - no matter what you make out of it :)

still the for two lines can be replaced

still the `for` two lines can be replaced
Outdated
Review

ah

ah
to_download = []
to_delete = []
Ghost marked this conversation as resolved
Review

old_files.get(entry["name"]) != ..., will return None if missing.

`old_files.get(entry["name"]) != ...`, will return `None` if missing.
Review

true, but an old database afine developer doesn't like comparing None with strings :)

true, but an old database afine developer doesn't like comparing None with strings :)
if current_rev:
for entry in current_rev.files_list(db):
old_files[entry["name"]] = f"{entry['md5']}-{entry['size']}"
old_files = {
Ghost marked this conversation as resolved Outdated
to_download = [e["name"] for e in self.files_list(db) if old_files.get(e["name"]) != f"{e['md5']}-{e['size']}"]
logging.debug(f"Downloading {to_download}")
```python to_download = [e["name"] for e in self.files_list(db) if old_files.get(e["name"]) != f"{e['md5']}-{e['size']}"] logging.debug(f"Downloading {to_download}") ```
Outdated
Review

That's outputting an array, which I don't want.

That's outputting an array, which I don't want.

right, the last line can be replaced with a for, but this change is for the for and if lines

right, the last line can be replaced with a `for`, but this change is for the `for` and `if` lines
Outdated
Review

What would be the benefit? I don't see it as more readable

What would be the benefit? I don't see it as more readable
e["name"]: f"{entry['md5']}-{entry['size']}"
for e in current_rev.files_list(db)
}
else:

same

same
old_files = dict()
for entry in self.files_list(db):
if old_files.get(entry["name"], "") != f"{entry['md5']}-{entry['size']}":
if old_files.get(entry["name"]) != f"{entry['md5']}-{entry['size']}":
logging.debug(f"Download {entry['name']}")
to_download.append(entry["name"])
to_download.append((entry["name"], entry["md5"]))
old_files.pop(entry["name"], None)
for entry in old_files.keys():
logging.debug(f"Delete {entry}")