Skip to content

Clear node collection cache after collection is done#6491

Merged
nicoddemus merged 1 commit intopytest-dev:featuresfrom
nicoddemus:clear-node-collection-cache
Jan 22, 2020
Merged

Clear node collection cache after collection is done#6491
nicoddemus merged 1 commit intopytest-dev:featuresfrom
nicoddemus:clear-node-collection-cache

Conversation

@nicoddemus
Copy link
Member

Also rename the variable to convey its intent better

Copy link
Contributor

@blueyed blueyed left a comment

Choose a reason for hiding this comment

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

I think the renaming is not necessary, and if so should go with type hints then at least.

@nicoddemus
Copy link
Member Author

I think the renaming is not necessary

I find it makes the intent of the cache clearer (only meant to use during collection); unless you are against it I would like to keep it.

I will add the type hints, thanks. 👍

@nicoddemus nicoddemus force-pushed the clear-node-collection-cache branch from 284f5b6 to 7d09db9 Compare January 17, 2020 12:59
@nicoddemus nicoddemus force-pushed the clear-node-collection-cache branch from 7d09db9 to f84157e Compare January 17, 2020 19:10
@nicoddemus
Copy link
Member Author

Done, thanks for the pointers!

Had to add a few more type hints and also applied the same logic to the old _pkg_roots attribute.

I want to eventually refactor the entire logic out of the Session class so the session instance is not so polluted with temporary variables/caches.

Copy link
Member

@bluetech bluetech left a comment

Choose a reason for hiding this comment

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

Looks like a good change to me.

Is this meant to fix a correctness issue or just optimization/reduce memory usage?

Also rename the involved variables to convey its intent better and
add type hints
@nicoddemus nicoddemus force-pushed the clear-node-collection-cache branch from f84157e to 7b1e3d1 Compare January 21, 2020 10:29
@nicoddemus nicoddemus changed the base branch from master to features January 21, 2020 10:29
@nicoddemus
Copy link
Member Author

Is this meant to fix a correctness issue or just optimization/reduce memory usage?

The latter. 😁

@nicoddemus
Copy link
Member Author

Rebased this on features too, as it should be.

blueyed added a commit to blueyed/pytest that referenced this pull request Jan 21, 2020
blueyed added a commit to blueyed/pytest that referenced this pull request Jan 21, 2020
@nicoddemus nicoddemus merged commit e17f5fa into pytest-dev:features Jan 22, 2020
@nicoddemus nicoddemus deleted the clear-node-collection-cache branch January 22, 2020 19:09
blueyed added a commit to blueyed/pytest that referenced this pull request Jan 22, 2020
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