remove unused code and raise an excpetion for chown_dir

[Dert1129]

Summary by CodeRabbit

• Bug Fixes

  • File ownership update failures are now logged with the affected package path instead of causing unlogged errors.
  • Directory copying now preserves empty directories and skips files that already exist at the destination.

• Improvements

  • File-copy operations now report the number of files successfully copied.

[coderabbitai]

Walkthrough

chown_dir now logs ownership-change failures instead of propagating them without context. Directory-content copying was moved from a public class method into an internal helper used by copy_files.

Changes

DLU filesystem behavior

Layer / File(s)	Summary
Ownership error handling data_management/services/dlu_filesystem.py	chown_dir catches failures during package and recursive file ownership updates and logs the package path with the exception.
Internal directory copying data_management/services/dlu_filesystem.py	The class-level copy_directory_contents method is removed, and equivalent copying and counting logic is nested inside copy_files.

Possibly related PRs

• KPMP/libra-data#174: Introduces the directory-copying helper and rewrites copy_files around it.

🚥 Pre-merge checks | ✅ 2

✅ Passed checks (2 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch KPMP-6726_clean-up

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

data_management/services/dlu_filesystem.py (1)

115-116: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Rename the variable dir to avoid shadowing the built-in function.

Using dir as a variable name shadows the Python built-in function dir(). It is recommended to use dirname instead.

♻️ Proposed refactor

-                for dir in dirs:
-                    subdir_path = os.path.join(root, dir)
+                for dirname in dirs:
+                    subdir_path = os.path.join(root, dirname)

Source: Linters/SAST tools

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 30f70637-7cf2-43e0-8dc0-35ea51e2fbde

📥 Commits

Reviewing files that changed from the base of the PR and between 3d7f0f6 and ef1638f.

📒 Files selected for processing (1)

• data_management/services/dlu_filesystem.py

[coderabbitai]

🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win

Re-raise the exception after logging to prevent false successes.

The PR title indicates the intent is to raise an exception for chown_dir failures. Currently, the exception is caught and logged, but not re-raised. As shown in the context snippet from data_management/watch_files.py, downstream callers do not check for failure and will incorrectly proceed to insert file records and mark the package upload as a "success" even if chown_dir fails.

Additionally, you can use logger.exception to automatically include the stack trace instead of manually casting e to a string.

🐛 Proposed fix to re-raise the exception

-        except Exception as e:
-            logger.error("Error changing ownership of directory %s: %s", package_path, str(e))
+        except Exception:
+            logger.exception("Error changing ownership of directory %s", package_path)
+            raise

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	except Exception as e:
	logger.error("Error changing ownership of directory %s: %s", package_path, str(e))
	except Exception:
	logger.exception("Error changing ownership of directory %s", package_path)
	raise

🧰 Tools

🪛 Ruff (0.15.21)

[warning] 119-119: Do not catch blind exception: Exception

(BLE001)