Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Python 3.12 compat #463

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ jobs:
strategy:
fail-fast: false
matrix:
python-version: ['3.8', '3.9', '3.10', '3.11']
python-version: ['3.8', '3.9', '3.10', '3.11', '3.12']

steps:
- uses: actions/checkout@v3
Expand Down
6 changes: 3 additions & 3 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
repos:
- repo: https://github.com/humitos/mirrors-autoflake
rev: v1.1
- repo: https://github.com/PyCQA/autoflake
rev: v2.3.1
hooks:
- id: autoflake
args: ['-i', '--remove-all-unused-imports']
Expand All @@ -19,7 +19,7 @@ repos:
hooks:
- id: reorder-python-imports
- repo: https://github.com/pycqa/flake8
rev: 6.0.0
rev: 6.1.0
hooks:
- id: flake8
- repo: https://github.com/mgedmin/check-python-versions
Expand Down
244 changes: 177 additions & 67 deletions artifactory.py
Original file line number Diff line number Diff line change
Expand Up @@ -422,7 +422,13 @@ def quote_url(url):
return quoted_url


class _ArtifactoryFlavour(pathlib._Flavour):
if sys.version_info.major == 3 and sys.version_info.minor <= 11:
parent_class = pathlib._Flavour
else:
parent_class = object


class _ArtifactoryFlavour(parent_class):
auburus marked this conversation as resolved.
Show resolved Hide resolved
"""
Implements Artifactory-specific pure path manipulations.
I.e. what is 'drive', 'root' and 'path' and how to split full path into
Expand All @@ -432,7 +438,7 @@ class _ArtifactoryFlavour(pathlib._Flavour):
drive: in context of artifactory, it's the base URI like
http://mysite/artifactory

root: repository, e.g. 'libs-snapshot-local' or 'ext-release-local'
root: like in unix, / when absolute, empty when relative

path: relative artifact path within the repository
"""
Expand All @@ -458,13 +464,6 @@ def join_parsed_parts(self, drv, root, parts, drv2, root2, parts2):
drv, root, parts, drv2, root2, parts2
)

if not root2 and len(parts2) > 1:
root2 = self.sep + parts2.pop(1) + self.sep

# quick hack for https://github.com/devopshq/artifactory/issues/29
# drive or repository must start with / , if not - add it
if not drv2.endswith("/") and not root2.startswith("/"):
drv2 = drv2 + self.sep
return drv2, root2, parts2

def splitroot(self, part, sep=sep):
Expand Down Expand Up @@ -501,7 +500,7 @@ def splitroot(self, part, sep=sep):

if url.path is None or url.path == sep:
if url.scheme:
return part.rstrip(sep), "", ""
return part.rstrip(sep), "/", ""
return "", "", part
elif url.path.lstrip("/").startswith("artifactory"):
mark = sep + "artifactory" + sep
Expand All @@ -510,8 +509,8 @@ def splitroot(self, part, sep=sep):
path = self._get_path(part)
drv = part.rpartition(path)[0]
path_parts = path.strip(sep).split(sep)
root = sep + path_parts[0] + sep
rest = sep.join(path_parts[1:])
root = sep
rest = sep.join(path_parts[0:])
Comment on lines +511 to +512
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are all these changes needed for Python 3.12 compatibility?

Can we limit the PR to just the necessary updates? This will make it easier to debug if it causes any regression issues.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried addressing this in the PR description because I completely understand this might be a blocker. Let me try explaining it again, and I'll be happy to try any suggestion you may have to avoid this set of changes (I couldn't think of one, apologies).

When I tried just doing the changes for 3.12 compatibility, one of the tests that broke was the one that relied on this being true:

ArtifactoryPath('http://b.com/artifactory/repo/path/file.txt').is_relative_to('http://b.com/artifactory')

This statement returns true in python3.11 but returns false in python3.12, and my guess is that we are relying on undefined behavior, based on how PureWindowsPath works.

# python 3.12
>>> PureWindowsPath("C://abc/def.txt").is_relative_to("C:")
False

So I felt I had two choices: Override most PurePath methods for artifactory, so it would behave as it used to in python3.11, or change ArtifactoryPath to consider / as root, instead of the repo. I thought the first option would be a maintenance nightmare, so that is why I went with the second one, but I could be wrong.

Happy to hear your thoughts on the subject!

(Also, all the changes for this new behavior are part of the first commit, happy to push that as a separate pull request if you think it would make things easier)

return drv, root, rest

if len(parts) >= 2:
Expand All @@ -524,14 +523,14 @@ def splitroot(self, part, sep=sep):
rest = part

if not rest:
return drv, "", ""
return drv, "/", ""

if rest == sep:
return drv, "", ""
return drv, "/", ""

if rest.startswith(sep):
root, _, part = rest[1:].partition(sep)
root = sep + root + sep
root = sep
part = rest.lstrip("/")

return drv, root, part

Expand Down Expand Up @@ -587,6 +586,29 @@ def make_uri(self, path):
"""
return path

def normcase(self, path):
return path

def splitdrive(self, path):
drv, root, part = self.splitroot(path)
return (drv + root, self.sep.join(part))

# This function is consumed by PurePath._load_parts() after python 3.12
def join(self, path, *paths):
drv, root, part = self.splitroot(path)

for next_path in paths:
drv2, root2, part2 = self.splitroot(next_path)
if drv2 != "":
drv, root, part = drv2, root2, part2
continue
if root2 != "":
root, part = root2, part2
continue
part = part + self.sep + part2

return drv + root + part


class _ArtifactorySaaSFlavour(_ArtifactoryFlavour):
def _get_base_url(self, url):
Expand Down Expand Up @@ -877,7 +899,11 @@ def get_stat_json(self, pathobj, key=None):
)
code = response.status_code
text = response.text
if code == 404 and ("Unable to find item" in text or "Not Found" in text or "File not found" in text):
if code == 404 and (
"Unable to find item" in text
or "Not Found" in text
or "File not found" in text
):
raise OSError(2, f"No such file or directory: {url}")

raise_for_status(response)
Expand Down Expand Up @@ -1479,6 +1505,21 @@ class PureArtifactoryPath(pathlib.PurePath):
_flavour = _artifactory_flavour
__slots__ = ()

def _init(self, *args):
super()._init(*args)

@classmethod
def _split_root(cls, part):
cls._flavour.splitroot(part)

@classmethod
def _parse_parts(cls, parts):
return super()._parse_parts(parts)

@classmethod
def _format_parsed_parts(cls, drv, root, tail):
return super()._format_parsed_parts(drv, root, tail)


class _FakePathTemplate(object):
def __init__(self, accessor):
Expand All @@ -1505,65 +1546,124 @@ class ArtifactoryPath(pathlib.Path, PureArtifactoryPath):
# in 3.9 and below Pathlib limits what members can be present in 'Path' class
__slots__ = ("auth", "verify", "cert", "session", "timeout")

def __new__(cls, *args, **kwargs):
"""
pathlib.Path overrides __new__ in order to create objects
of different classes based on platform. This magic prevents
us from adding an 'auth' argument to the constructor.
So we have to first construct ArtifactoryPath by Pathlib and
only then add auth information.
"""
obj = pathlib.Path.__new__(cls, *args, **kwargs)
if sys.version_info.major == 3 and sys.version_info.minor <= 11:
auburus marked this conversation as resolved.
Show resolved Hide resolved

cfg_entry = get_global_config_entry(obj.drive)
def __new__(cls, *args, **kwargs):
"""
In python3.12 this is no longer needed

# Auth section
apikey = kwargs.get("apikey")
token = kwargs.get("token")
auth_type = kwargs.get("auth_type")
pathlib.Path overrides __new__ in order to create objects
of different classes based on platform. This magic prevents
us from adding an 'auth' argument to the constructor.
So we have to first construct ArtifactoryPath by Pathlib and
only then add auth information.
"""

if apikey:
logger.debug("Use XJFrogApiAuth apikey")
obj.auth = XJFrogArtApiAuth(apikey=apikey)
elif token:
logger.debug("Use XJFrogArtBearerAuth token")
obj.auth = XJFrogArtBearerAuth(token=token)
else:
auth = kwargs.get("auth")
obj.auth = auth if auth_type is None else auth_type(*auth)
obj = pathlib.Path.__new__(cls, *args, **kwargs)

if obj.auth is None and cfg_entry:
auth = (cfg_entry["username"], cfg_entry["password"])
obj.auth = auth if auth_type is None else auth_type(*auth)
cfg_entry = get_global_config_entry(obj.drive)

obj.cert = kwargs.get("cert")
obj.session = kwargs.get("session")
obj.timeout = kwargs.get("timeout")
# Auth section
apikey = kwargs.get("apikey")
token = kwargs.get("token")
auth_type = kwargs.get("auth_type")

if obj.cert is None and cfg_entry:
obj.cert = cfg_entry["cert"]
if apikey:
logger.debug("Use XJFrogApiAuth apikey")
obj.auth = XJFrogArtApiAuth(apikey=apikey)
elif token:
logger.debug("Use XJFrogArtBearerAuth token")
obj.auth = XJFrogArtBearerAuth(token=token)
else:
auth = kwargs.get("auth")
obj.auth = auth if auth_type is None else auth_type(*auth)

if "verify" in kwargs:
obj.verify = kwargs.get("verify")
elif cfg_entry:
obj.verify = cfg_entry["verify"]
else:
obj.verify = True
if obj.auth is None and cfg_entry:
auth = (cfg_entry["username"], cfg_entry["password"])
obj.auth = auth if auth_type is None else auth_type(*auth)

if obj.session is None:
obj.session = requests.Session()
obj.session.auth = obj.auth
obj.session.cert = obj.cert
obj.session.verify = obj.verify
obj.session.timeout = obj.timeout
obj.cert = kwargs.get("cert")
obj.session = kwargs.get("session")
obj.timeout = kwargs.get("timeout")

return obj
if obj.cert is None and cfg_entry:
obj.cert = cfg_entry["cert"]

def _init(self, *args, **kwargs):
if "template" not in kwargs:
kwargs["template"] = _FakePathTemplate(_artifactory_accessor)
if "verify" in kwargs:
obj.verify = kwargs.get("verify")
elif cfg_entry:
obj.verify = cfg_entry["verify"]
else:
obj.verify = True

if obj.session is None:
obj.session = requests.Session()
obj.session.auth = obj.auth
obj.session.cert = obj.cert
obj.session.verify = obj.verify
obj.session.timeout = obj.timeout

return obj

def _init(self, *args, **kwargs):
if "template" not in kwargs:
kwargs["template"] = _FakePathTemplate(_artifactory_accessor)

super(ArtifactoryPath, self)._init(*args, **kwargs)

if sys.version_info.major == 3 and sys.version_info.minor >= 12:

def __init__(self, *args, **kwargs):
if "template" not in kwargs:
kwargs["template"] = _FakePathTemplate(_artifactory_accessor)

super().__init__(*args, **kwargs)

if sys.version_info.major == 3 and sys.version_info.minor >= 12:
# This behavior is in __init__ under python3.12, and in __new__
# in <python3.11

auburus marked this conversation as resolved.
Show resolved Hide resolved
cfg_entry = get_global_config_entry(self.drive)

# Auth section
apikey = kwargs.get("apikey")
token = kwargs.get("token")
auth_type = kwargs.get("auth_type")

super(ArtifactoryPath, self)._init(*args, **kwargs)
if apikey:
logger.debug("Use XJFrogApiAuth apikey")
self.auth = XJFrogArtApiAuth(apikey=apikey)
elif token:
logger.debug("Use XJFrogArtBearerAuth token")
self.auth = XJFrogArtBearerAuth(token=token)
else:
auth = kwargs.get("auth")
self.auth = auth if auth_type is None else auth_type(*auth)

if self.auth is None and cfg_entry:
auth = (cfg_entry["username"], cfg_entry["password"])
self.auth = auth if auth_type is None else auth_type(*auth)

self.cert = kwargs.get("cert")
self.session = kwargs.get("session")
self.timeout = kwargs.get("timeout")

if self.cert is None and cfg_entry:
self.cert = cfg_entry["cert"]

if "verify" in kwargs:
self.verify = kwargs.get("verify")
elif cfg_entry:
self.verify = cfg_entry["verify"]
else:
self.verify = True

if self.session is None:
self.session = requests.Session()
self.session.auth = self.auth
self.session.cert = self.cert
self.session.verify = self.verify
self.session.timeout = self.timeout

def __reduce__(self):
# pathlib.PurePath.__reduce__ doesn't include instance state, but we
Expand Down Expand Up @@ -1635,6 +1735,16 @@ def stat(self, pathobj=None):
pathobj = pathobj or self
return self._accessor.stat(pathobj=pathobj)

def exists(self):
try:
self.stat()
except OSError:
return False
except ValueError:
# Non-encodable path
return False
return True

def mkdir(self, mode=0o777, parents=False, exist_ok=False):
"""
Create a new directory at this given path.
Expand Down Expand Up @@ -2367,12 +2477,12 @@ def promote_docker_image(

@property
def repo(self):
return self._root.replace("/", "")
return self.parts[1]

@property
def path_in_repo(self):
parts = self.parts
path_in_repo = "/" + "/".join(parts[1:])
path_in_repo = "/" + "/".join(parts[2:])
return path_in_repo

def find_user(self, name):
Expand Down
1 change: 1 addition & 0 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
"Programming Language :: Python :: 3.9",
"Programming Language :: Python :: 3.10",
"Programming Language :: Python :: 3.11",
"Programming Language :: Python :: 3.12",
"Topic :: Software Development :: Libraries",
"Topic :: System :: Filesystems",
],
Expand Down
Loading
Loading