Fix: #13420 - Add cache for nodes._check_initialpaths_for_relpath #13422
Conversation
psf-chronographer
bot
added
the
bot:chronographer:provided
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
this looks good as a workaround - i recon the path types doo too much for their own good
the proposal in #10824 would be a more invasive solution to the intended problem as it would "root" node-ids in the collection root
| @@ -594,7 +598,7 @@ def __init__( | |||
| try: | |||
| nodeid = str(self.path.relative_to(session.config.rootpath)) | |||
| except ValueError: | |||
| nodeid = _check_initialpaths_for_relpath(session, path) | |||
| nodeid = _check_initialpaths_for_relpath(session._initialpaths, path) | |||
There was a problem hiding this comment.
@bluetech @nicoddemus seeing this right in place i'm thinking - what if they cooperated with the parent
with the introduction of directory nodes i believe the case is a lot more clear cut and possibly could avoid the common path calls all-together
also it seems like going for directory names more times could potentially safe a lot of time as commonly there are many files in the same directory
There was a problem hiding this comment.
what if they cooperated with the parent with the introduction of directory nodes i believe the case is a lot more clear cut and possibly could avoid the common path calls all-together
Not sure I follow, we already have Directory nodes:
Line 635 in cfbe319
sashko1988
changed the title
sashko1988
changed the title
I agree. This is rather a quick workaround that shouldn't do any harm. Don't know how long #10824 will take. But it would be good to see my fix in the next patch release. |
|
@RonnyPfannschmidt do I need to add anything else so Changelog entry check starts running? |
|
@webknjaz any idea why its down? |
|
Dunno. If it's a one-time occasion, might be a network error or something that prevented GH from sending or receiving data. We can retrigger it to see. |
webknjaz
added
skip news
|
Looks like it was a fluke 🤷♂️ |
|
@RonnyPfannschmidt @webknjaz folks, I might be blind, but I don't see the Merge button. Is there anything else missing? |
|
@sashko1988 only people with a commit bit see it. GH doesn't allow arbitrary users put code into arbitrary repos they don't own. |
|
@webknjaz, fair enough. Would you happen to know when this PR will be merged - if it will be merged at all? |
|
You got an approval from Ronny. Unless someone objects, he'll probably merge it soon. But it may hang for some while. |
|
Should this be backported? |
|
@RonnyPfannschmidt, yes, please! It would be good to have that fix in 8.3.x! |
RonnyPfannschmidt
added
the
backport 8.3.x
Backport to 8.3.x: 💚 backport PR created✅ Backport PR branch: Backported as #13425 🤖 @patchback |
…3422) * add lru_cache to nodes._check_initialpaths_for_relpath update tests * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: Oleksandr Zavertniev <oleksandr.zavertniev@yellowbrick.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> (cherry picked from commit cfbe319)
…3422) (#13425) * add lru_cache to nodes._check_initialpaths_for_relpath update tests * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- (cherry picked from commit cfbe319) Co-authored-by: Sashko <20253875+sashko1988@users.noreply.github.com> Co-authored-by: Oleksandr Zavertniev <oleksandr.zavertniev@yellowbrick.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
lru_cachetonodes._check_initialpaths_for_relpathfixes #13420