Allow for changing the spatial filter with kart checkout #465
Conversation
olsen232
force-pushed
the
spatial-filter-5
branch
from
eba0aa0 to
ad63b4a
Compare
kart/checkout.py
Outdated
|
|
||
| db_tree_matches = ( | ||
| working_copy.get_db_tree() == target_tree_or_commit.peel(pygit2.Tree).hex | ||
| ) | ||
|
|
||
| if discard_changes or not db_tree_matches: | ||
| if discard_changes or not db_tree_matches or not spatial_filter_matches: |
There was a problem hiding this comment.
we never hit this because of the above, so might as well remove it
kart/spatial_filters.py
Outdated
| ) | ||
|
|
||
| def resolve(self, repo): | ||
| # Returns the ResolvedSpatialFilterSpec that this ReferenceSpatialFilterSpec points to. |
There was a problem hiding this comment.
This comment needs clarifying I think (and turning into a docstring)
There was a problem hiding this comment.
Expanded the docstring and moved it to a superclass.
| except (KeyError, ValueError): | ||
| pass | ||
|
|
||
| if obj is not None: | ||
| return None, oid, self._resolve_object_contents(obj) | ||
|
|
||
| ref = self.ref_or_oid |
There was a problem hiding this comment.
So this function is assuming (AFAICT) that at this point the self.ref_or_oid is the name of a ref rather than an OID. Is that true? It seems like it could be an OID of an unfetched object also. Is there any way to tell?
There was a problem hiding this comment.
It does assume that if it doesn't find the object, it must be a ref - or missing. It could be that it is an unfetched object however. But whether non-existent or unfetched, the end results will be this:
No spatial filter object was found in the repository at 9d5d8e7 or refs/filters/9d5d8e7 - which is true and describes the problem accurately.
I'll add a TODO to handle unfetched objects here somehow, but it's part of a bigger problem - we can't currently tell if objects are non-existent / corrupted or merely unfetched, and we don't specifically handle unfetched objects anywhere.
kart/working_copy/base.py
Outdated
| if not force: | ||
| self.check_not_dirty() | ||
|
|
||
| self.drop_table(commit, *datasets) |
| @@ -1010,6 +1046,18 @@ def _delete_meta(self, sess, dataset): | |||
| """ | |||
| raise NotImplementedError() | |||
|
|
|||
| def rewrite_full(self, commit, *datasets, force=False): | |||
There was a problem hiding this comment.
should this method be done in one transaction? Otherwise a client watching carefully will see tables be dropped, and then recreated shortly afterwards. For big WCs those could be a while apart.
Description
kart checkout --spatial-filter=SPECnow updates the spatial filter. The syntax is the same as forkart initandkart clone.The spatial filter cannot be changed while there are uncommitted changes in the working copy (this avoids certain potential problems with handling probably-accidental-PK-collisions - note that these aren't actually handled yet however), and it works by rewriting the entire working copy from scratch (anything else would be an optimisation, and would only speed things up in the case that the intersection between the old and the new spatial filter is large compared to the their total size).
Not done in this PR:
Related links:
#456