Skip to content

Change Paths to use pathlib.Path#1431

Merged
hmstepanek merged 12 commits intomainfrom
feature-pathlib
Jul 28, 2025
Merged

Change Paths to use pathlib.Path#1431
hmstepanek merged 12 commits intomainfrom
feature-pathlib

Conversation

@TimPansino
Copy link
Copy Markdown
Contributor

@TimPansino TimPansino commented Jul 23, 2025

Overview

To make paths more portable for Windows support:

  • Update all paths in the agent from strings and os.path to use pathlib.Path.
  • Enable flake8-pathlib linter to ensure no regressions.
  • Update ruff to latest version and ignore new violations about module import order.

@TimPansino TimPansino requested a review from a team as a code owner July 23, 2025 22:56
@github-actions
Copy link
Copy Markdown

github-actions bot commented Jul 23, 2025

🦙 MegaLinter status: ✅ SUCCESS

Descriptor Linter Files Fixed Errors Warnings Elapsed time
✅ ACTION actionlint 6 0 0 0.73s
✅ MARKDOWN markdownlint 7 0 0 0 1.12s
✅ MARKDOWN markdown-link-check 7 0 0 11.88s
✅ PYTHON ruff 922 0 0 0 0.85s
✅ PYTHON ruff-format 922 0 0 0 0.3s
✅ YAML prettier 13 0 0 0 1.22s
✅ YAML v8r 13 0 0 6.18s
✅ YAML yamllint 13 0 0 0.61s

See detailed report in MegaLinter reports

MegaLinter is graciously provided by OX Security

@mergify mergify bot added the tests-failing Tests failing in CI. label Jul 23, 2025
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Jul 23, 2025

Codecov Report

Attention: Patch coverage is 72.50000% with 11 lines in your changes missing coverage. Please review.

Project coverage is 81.36%. Comparing base (777e025) to head (eb8ff6c).

Files with missing lines Patch % Lines
newrelic/bootstrap/sitecustomize.py 0.00% 7 Missing ⚠️
newrelic/common/system_info.py 80.00% 1 Missing and 1 partial ⚠️
newrelic/__init__.py 75.00% 1 Missing ⚠️
newrelic/common/agent_http.py 50.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1431   +/-   ##
=======================================
  Coverage   81.35%   81.36%           
=======================================
  Files         208      208           
  Lines       23554    23563    +9     
  Branches     3716     3716           
=======================================
+ Hits        19162    19171    +9     
+ Misses       3134     3133    -1     
- Partials     1258     1259    +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

hmstepanek
hmstepanek previously approved these changes Jul 24, 2025
Copy link
Copy Markdown
Contributor

@hmstepanek hmstepanek left a comment

Choose a reason for hiding this comment

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

Had a couple questions, otherwise looks good!

# If it is a buildout created script, it will replace the whole
# sys.path again later anyway.
root_directory = os.path.dirname(os.path.dirname(boot_directory))
root_directory = str(Path(boot_directory).parent.parent)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is actually cleaner code than it was before which is nice. 😊


try:
pseudoDevices = os.listdir("/devices/pseudo/")
pseudoDevices = Path("/devices/pseudo/").iterdir()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't think these locations make sense on windows but I assume we'll address that later.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No but according to the commend above in this file, that line is not reachable as Windows is already handled. That block is only for checking on Solaris systems I think based on the other comment.

    # The multiprocessing module provides support for Windows,
    # BSD systems (including MacOS X) and systems which support
    # the POSIX API for querying the number of CPUs.

    try:
        return multiprocessing.cpu_count()
    except NotImplementedError:
        pass
    # Assuming that Solaris will support POSIX API for querying
    # the number of CPUs. Just in case though, work it out by
    # looking at the devices corresponding to the available CPUs.

    try:
        pseudoDevices = os.listdir("/devices/pseudo/")


atexit.register(self.__socket_cleanup, self.__listener_socket)
os.chmod(self.__listener_socket, 0o600)
Path(self.__listener_socket).chmod(0o600)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Does this work on windows?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sounds like no, but it'll ignore it. Can try and fix that when we get to trying to run this on Windows if it's necessary to fix.

https://docs.python.org/3.13/library/os.html#os.chmod

uwsgi_dir = temp_dir / "uwsgi"
init_file = uwsgi_dir / "__init__.py"
uwsgi_dir.mkdir(parents=True)
with init_file.open("w") as f:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is the default mode when opening a file the same between os.open and pathlib.open?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@mergify mergify bot removed the tests-failing Tests failing in CI. label Jul 24, 2025
Copy link
Copy Markdown
Contributor

@hmstepanek hmstepanek left a comment

Choose a reason for hiding this comment

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

Looks good!

@hmstepanek hmstepanek merged commit b089b33 into main Jul 28, 2025
60 checks passed
@hmstepanek hmstepanek deleted the feature-pathlib branch July 28, 2025 16:46
@TimPansino TimPansino added this to the v10.16.0 milestone Aug 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants