Remove pyfs dependency#134
Conversation
| os.system("dd bs=1024 count={1} </dev/urandom >{0}".format(filepath, 1024 * 10)) | ||
| # Create test file with random data using subprocess | ||
| count = 1024 * 10 | ||
| subprocess.check_call(["dd", "bs=1024", f"count={count}", "if=/dev/urandom", f"of={filepath}"]) |
There was a problem hiding this comment.
os.system is deprecated
| t1 = time.time() | ||
|
|
||
| fs = open_fs(url) | ||
| fs = XRootDPyFS(url) |
There was a problem hiding this comment.
the Opened has been removed
| @@ -0,0 +1,1696 @@ | |||
| # SPDX-FileCopyrightText: 2015, 2023 CERN. | |||
There was a problem hiding this comment.
Copied from pyfs source code
There was a problem hiding this comment.
Hmm if it's largely copied then we should preserve the original copyright/license notice 😃
There was a problem hiding this comment.
Good suggestion, thank you. When the funcs have been copied, I checked license (MIT) and copyright headers. The latter are not there. I have added a new line with a reference to the license and repo.
cfa68ae to
6b7a18d
Compare
kpsherva
left a comment
kpsherva
left a comment
There was a problem hiding this comment.
LGTM!
Sorry for jumping in late with this question, but I understand that copying pyfs utils functions was the only way to go, right? were there any alternative solutions?
zzacharo
left a comment
zzacharo
left a comment
There was a problem hiding this comment.
LGTM! I guess the pyfs needs to be removed from invenio-xrootd too?
OliverGeneser
left a comment
OliverGeneser
left a comment
There was a problem hiding this comment.
I will test it a bit more, but I thing it's good 👍
| @@ -0,0 +1,1696 @@ | |||
| # SPDX-FileCopyrightText: 2015, 2023 CERN. | |||
There was a problem hiding this comment.
Hmm if it's largely copied then we should preserve the original copyright/license notice 😃
slint
left a comment
slint
left a comment
There was a problem hiding this comment.
LGTM from a brief pass. I think we're going to have other places in the code where we just didn't use the PyFS features in our paths, but I don't think it makes sense to trim down this compat module for this reason.
I still want to test "live" with EOS, but have to find time to setup...
|
@slint indeed, we need to test this in a live system. We can release an alpha, fix and release |
5ddb740 to
1c4d9f2
Compare
- remove the dependency on pyfs Pyfilesystem2, as it has been removed from Invenio packages and it blocks Python upgrade - remove the pyfs Opener - add pyfs required classes and utilities in the package
|
I will merge this so we can go ahead with testing, I can fix any comment in a subsequent PR. |
5 participants
Pyfs dependency has been removed from the Invenio ecosystem.
This pull requests removes the pyfs (Pyfilesystem2) dependency, adds the necessary classes and utilities to the package and removes the Opener (the only breaking change).