Skip to content

Migrate from os.path to pathlib for file path handling #654

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.

Sign up for GitHub

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

Open
zliang-akamai wants to merge 1 commit into
base: dev
Choose a base branch
from
+22 −19
Open

Migrate from os.path to pathlib for file path handling #654

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 3 additions & 3 deletions docs/conf.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,11 @@

# If extensions (or modules to document with autodoc) are in another directory,
# add these directories to sys.path here. If the directory is relative to the
# documentation root, use os.path.abspath to make it absolute, like shown here.
# documentation root, use Path(...).resolve() to make it absolute, like shown here.
#
import os
import sys
sys.path.insert(0, os.path.abspath('..'))
from pathlib import Path
sys.path.insert(0, str(Path('..').absolute()))
Copy link

Copilot AI Feb 28, 2026

Choose a reason for hiding this comment

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

In docs/conf.py the comment says to use Path(...).resolve() but the code uses Path('..').absolute(). Either update the code to use resolve() (and keep the comment accurate) or adjust the comment to match the chosen API to avoid misleading future edits.

Suggested change
sys.path.insert(0, str(Path('..').absolute()))
sys.path.insert(0, str(Path('..').resolve()))

Copilot uses AI. Check for mistakes.


# -- Project information -----------------------------------------------------
Expand Down
8 changes: 4 additions & 4 deletions linode_api4/common.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import os
from dataclasses import dataclass
from pathlib import Path

from linode_api4.objects import JSONObject

Expand Down Expand Up @@ -47,9 +47,9 @@ def load_and_validate_keys(authorized_keys):
ret.append(k)
else:
# it doesn't appear to be a key.. is it a path to the key?
k = os.path.expanduser(k)
if os.path.isfile(k):
with open(k) as f:
k_path = Path(k).expanduser()
if k_path.is_file():
with open(k_path) as f:
ret.append(f.read().rstrip())
Comment on lines +50 to 53
Copy link

Copilot AI Feb 28, 2026

Choose a reason for hiding this comment

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

The pathlib migration here changes the file-path handling branch, but unit tests only cover the "raw key" case (not loading the key from a file). Adding a unit test that passes a temp file path (e.g., via pytest tmp_path) would help ensure the Path(...).expanduser().is_file() + read behavior stays correct across platforms.

Copilot uses AI. Check for mistakes.
else:
raise ValueError(
Expand Down
7 changes: 4 additions & 3 deletions linode_api4/groups/linode.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import base64
import os
from pathlib import Path
from typing import Any, Dict, List, Optional, Union

from linode_api4.common import load_and_validate_keys
Expand Down Expand Up @@ -457,8 +457,9 @@ def stackscript_create(
script_body = script
if not script.startswith("#!"):
# it doesn't look like a stackscript body, let's see if it's a file
if os.path.isfile(script):
with open(script) as f:
script_path = Path(script)
if script_path.is_file():
with open(script_path) as f:
script_body = f.read()
Comment on lines +460 to 463
Copy link

Copilot AI Feb 28, 2026

Choose a reason for hiding this comment

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

The file-path branch of stackscript_create is now implemented with pathlib, but there doesn’t appear to be unit test coverage verifying that providing script as a path loads the file contents. Adding a test that writes a temporary script file and passes its path would guard against regressions in this behavior.

Copilot uses AI. Check for mistakes.
else:
raise ValueError(
Expand Down
8 changes: 4 additions & 4 deletions linode_api4/groups/profile.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import os
from datetime import datetime
from pathlib import Path

from linode_api4 import UnexpectedResponseError
from linode_api4.common import SSH_KEY_TYPES
Expand Down Expand Up @@ -322,9 +322,9 @@ def ssh_key_upload(self, key, label):
"""
if not key.startswith(SSH_KEY_TYPES):
# this might be a file path - look for it
path = os.path.expanduser(key)
if os.path.isfile(path):
with open(path) as f:
key_path = Path(key).expanduser()
if key_path.is_file():
with open(key_path) as f:
key = f.read().strip()
Comment on lines +325 to 328
Copy link

Copilot AI Feb 28, 2026

Choose a reason for hiding this comment

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

This update switches the "treat input as a file path" branch to pathlib, but the existing unit test(s) for ssh_key_upload appear to only cover passing a literal SSH key string. Consider adding a unit test that writes a key to a temp file and passes that file path to verify the pathlib-based file detection and reading behavior.

Copilot uses AI. Check for mistakes.
if not key.startswith(SSH_KEY_TYPES):
raise ValueError("Invalid SSH Public Key")
Expand Down
12 changes: 7 additions & 5 deletions linode_api4/objects/nodebalancer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import os
from pathlib import Path
from urllib import parse

from linode_api4.common import Price, RegionPrice
Expand Down Expand Up @@ -220,12 +220,14 @@ def load_ssl_data(self, cert_file, key_file):

# we're disabling warnings here because these attributes are defined dynamically
# through linode.objects.Base, and pylint isn't privy
if os.path.isfile(os.path.expanduser(cert_file)):
with open(os.path.expanduser(cert_file)) as f:
cert_path = Path(cert_file).expanduser()
if cert_path.is_file():
with open(cert_path) as f:
self.ssl_cert = f.read()

if os.path.isfile(os.path.expanduser(key_file)):
with open(os.path.expanduser(key_file)) as f:
key_path = Path(key_file).expanduser()
if key_path.is_file():
with open(key_path) as f:
self.ssl_key = f.read()


Expand Down