Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Injector stats #161

Merged
merged 20 commits into from
Sep 13, 2023
Merged

Injector stats #161

merged 20 commits into from
Sep 13, 2023

Conversation

proway2
Copy link
Contributor

@proway2 proway2 commented Sep 13, 2023

Fixes #158

  • For every internal dependency that's being injected there will be a record in Scrapy stats:
poet/injector/tests.test_injection.PricePO 15
  • Other places in the injector, where stats are produced, have updated prefix poet/*
  • pytest-twisted is added to requirements-dev.txt otherwise it's impossible to set up the local env (tox works fine in any case)

@codecov
Copy link

codecov bot commented Sep 13, 2023

Codecov Report

Merging #161 (4c30856) into master (50c4578) will increase coverage by 0.05%.
The diff coverage is 100.00%.

❗ Current head 4c30856 differs from pull request most recent head 0ab0e40. Consider uploading reports for the commit 0ab0e40 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #161      +/-   ##
==========================================
+ Coverage   85.62%   85.67%   +0.05%     
==========================================
  Files          14       14              
  Lines         800      803       +3     
==========================================
+ Hits          685      688       +3     
  Misses        115      115              
Files Changed Coverage Δ
scrapy_poet/injection.py 98.49% <100.00%> (+0.02%) ⬆️

Copy link
Member

@Gallaecio Gallaecio 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 to me.

I do wonder if we should mention in the Stats page of the documentation the stats that are logged beyond those that are created using web-poet stats. Or maybe just mention the kind of stuff that’s logged, without going into detail.

@proway2
Copy link
Contributor Author

proway2 commented Sep 13, 2023

@Gallaecio I'll add a short note to docs, without details though.

@proway2
Copy link
Contributor Author

proway2 commented Sep 13, 2023

@Gallaecio when this could potentially be merged and released?

tests/test_injection.py Outdated Show resolved Hide resolved
@kmike kmike merged commit 3706836 into scrapinghub:master Sep 13, 2023
13 checks passed
@kmike
Copy link
Member

kmike commented Sep 13, 2023

Thanks @proway2!

@proway2 proway2 deleted the po-stats branch September 14, 2023 07:06
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.

Stats for POs
3 participants