Also looks like it needs to be rebased against the current main branch. Thanks.

So looks like an existing functional test is failing because it expects the return value of headers to be a dict rather than a ProjectFile. Should I conform to that behavior or should I update the test based on me now constructing a ProjectFile from the metadata? I assume the former but I feel like the latter is more usable, but arguably a breaking change.

https://github.com/python-gitlab/python-gitlab/blob/main/tests/functional/api/test_repository.py#L43

So looks like an existing functional test is failing because it expects the return value of headers to be a dict rather than a ProjectFile. Should I conform to that behavior or should I update the test based on me now constructing a ProjectFile from the metadata? I assume the former but I feel like the latter is more usable, but arguably a breaking change.

https://github.com/python-gitlab/python-gitlab/blob/main/tests/functional/api/test_repository.py#L43

@holysoles our client's http_head method (and also head()) returns just the headers dict without really transforming them, so I think we should either keep it consistent with that, or change it everywhere. IMO it's best to leave this as-is for now, so it's a bit more generic, and discuss that for all head methods in a follow-up.

But I'm a bit confused our existing tests work as intended as opposed to your issue description, it seems like the issue there is actually your use of keyword args, but I'll have to check in more detail when I get home.

@nejch now that you say that, the keyword args do seem like the problem. I think my confusion initially stemmed from the official REST API docs indicate that there isnt a difference in how the API call should be made, but here, the user needs to pass id=my_file_path compared to file_path=my_file_path.

Given all this, does it make sense to not transform the headers dict and just return it, but still proceed with the other changes so the consistency of the parameter name can be improved?

@nejch now that you say that, the keyword args do seem like the problem. I think my confusion initially stemmed from the official REST API docs indicate that there isnt a difference in how the API call should be made, but here, the user needs to pass id=my_file_path compared to file_path=my_file_path.

Given all this, does it make sense to not transform the headers dict and just return it, but still proceed with the other changes so the consistency of the parameter name can be improved?

Yes, I think that makes sense. TBH I'm questioning whether we really need to even be overriding the GetMixin for the file manager because we technically already kind of cover this via the _id_attr = "file_path" and we could just document usage, but this is historical and not part of this PR.

For now IMO best to just align the head() override with the get() one and keep whatever is returned from http_head. They are HTTP headers in the end and not really one-to-one replacement for ProjectFile attributes so this could also be confusing in some cases otherwise. WDYT?

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.61%. Comparing base (10c7aee) to head (5afd332).
Report is 1 commits behind head on main.

@@           Coverage Diff           @@
##             main    #2977   +/-   ##
=======================================
  Coverage   96.60%   96.61%           
=======================================
  Files          95       95           
  Lines        6072     6080    +8     
=======================================
+ Hits         5866     5874    +8     
  Misses        206      206