Before, there was no clean way to feed a qswhid to a json encoder.

This will be useful to reuse them in other tests.

Before this patch giving `from_disk` and directory path with final '/'s
would confuses the exclusion code as it would assume it need to exclude
one extra character to get the basename. This would swallow the first
char of the directory to exclude, leading to a righful crash.

This would affect directory excluded in the first directory only as sub
directory would be called without a '/'.

We fix the path at the top of the function as trailing '/'s could
confuse other code too in the future.

Using os.walk() does not make much sense when we want to control what
directories to recurse into.

Additionally, this uses os.scandir directly, which allows us to directly
sort symlinks and files apart from directories (while os.walk groups
symlinks with directories) without two extra system calls.

This creates a tree structure from an easy to read textual
representation of said structure.

It used to traverse all the directory and filter elements of said tree
only afterwards; this version should be a bit smarter and not go too far
deep in directories that should be ignored.

We cannot just use the subtree eviction mechanism of
'os.walk(topdown=True)' because the filtering callback takes some
context of the subdirectory content (typically to be able to evict empty
directories).

This version of the code is a bit more complex but should do the trick.

Pierre-Yves David authored 9 months ago and Pierre-Yves David committed 9 months ago

git_objects.snapshot_git_object wants Snapshot argument, lets move
things to Snapshot object.

Pierre-Yves David authored 9 months ago and Pierre-Yves David committed 9 months ago

object_dicts still need to be migrated.

Pierre-Yves David authored 9 months ago and Pierre-Yves David committed 9 months ago

And automatically generate it as we are at it.

Pierre-Yves David authored 9 months ago and Pierre-Yves David committed 9 months ago

We also take this as an opportunity to use the blacklist_types feature
in that test.

Pierre-Yves David authored 9 months ago and Pierre-Yves David committed 9 months ago

We also typed the function, which turned out more painful than
anticipated.

Pierre-Yves David authored 9 months ago and vlorentz committed 9 months ago

This test work with cpython because as we do not assign the open file to
a variable, the reference counting garbage collect it right after the
`write` call. Deleting the file object mean the write is flushed.

On an interpreter without reference counting, as pypy, the file object might be
garbaged collected too late, breaking the test.

So we give the file object a clear life cycle.

Pierre-Yves David authored 9 months ago and vlorentz committed 9 months ago

As for the previous commit, using deprecated actually change the source
Class leading all usage to raise deprecation warning. So we have to
remove the deprecated and keep the compatibility silently for a small
while.

Pierre-Yves David authored 9 months ago and vlorentz committed 9 months ago

It turns out that calling @deprecated on Content alter the class, and
any instantion of "Content" will be wrongly marked as deprecated… So use
a different approach too preserve compatibility (and we won't keep it
long).

It do works, but it is now discouraged.

Pierre-Yves David authored 10 months ago and vlorentz committed 10 months ago

TargetType is far too generic and introduces confusion with
ReleaseTargetType. So we rename it to a clearer name.

Pierre-Yves David authored 10 months ago and vlorentz committed 10 months ago

ObjectType is far too generic and introduces confusion with
swhids.ObjectType. So we rename it to a clearer name.

A Content object with no attached data or get_data function could
no longer be converted to a dict as MissingData exception was raised.

vlorentz authored 10 months ago and Antoine Lambert committed 10 months ago

Having the escaped URL in `swhid.origin` is inconsistent with self.path
(which is always escaped) and never what we want, because it is only
useful while serializing, which is already handled by `__str__`.

This led to swh-indexer#4738
where swh-deposit parsed a qualified SWHID, then used `.origin` to get
an origin URL.

Additionally, as serialization always escapes the `origin` qualifier,
this means that deserializing then re-serializing a qualified SWHID
would double-escape it.

Finally, fixing this made the test uncover that `%` was not escaped
while serializing, while `;` was, leading to incorrect (and ambiguous)
escaped URLs.

There are two other package using DiskBackedContent "swh-loader-svn" and
"swh-loader-cvs". Both use it to check "DiskBackedContent.object_type"
at the same time as "model.Content.object_type".

so we do this small hack to avoid breaking these other module until
they migrate.

This sets the pieces in place to finally cleanup the confusion from the
various object_type attributes. They now have different type, so we
should be able to start detecting error at some point.

As for FromDiskType, we keep compatibility with string value for now.
This avoid breaking existing code.

Instead of having multiple class and `object_type` value, we just adds
a few lines in the main `model.Content` class to retrieved data on
demand. The `with_data` logic already existed there anyway.

This will avoid having from_disk extending the model from the outside.

This is part of a wider effort to differentiate the various type of
"object_type" attribute around the model code base.

This requires cleaning various item along the ways. Which is probably an
added benefit. Especially, mypy now consider BaseModel to hold a
object_type attribute.

Check inline comment for details.

Currently the only limit is "enforced" by PostgreSQL.

This makes sure that origins created after we switch to Cassandra as the
primary storage remain compatible with a PostgreSQL-based storage.