Skip to content

Cleanup#104

Open
elacuesta wants to merge 9 commits intomasterfrom
cleanup
Open

Cleanup#104
elacuesta wants to merge 9 commits intomasterfrom
cleanup

Conversation

@elacuesta
Copy link
Member

Several unrelated cleanups:

  • Remove from future import print_function
  • Always use importlib.metadata instead of pkg_resources
  • Replace deprecated utcnow calls
  • Remove object parent class
  • Modernize super() calls
  • Remove python2-only tests
  • Always use argparse

Client(_sentry_dsn).captureException()
except Exception as err:
print(datetime.datetime.utcnow().isoformat(),
print(dt.now(timezone.utc).isoformat(),
Copy link
Member Author

Choose a reason for hiding this comment

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

Technically not exactly the same, the new version adds +00:00:

In [19]: dt.now(timezone.utc).isoformat()
Out[19]: '2026-02-25T16:09:38.206121+00:00'

In [20]: dt.utcnow().isoformat()
<ipython-input-20-cac1a3b575b4>:1: DeprecationWarning: datetime.datetime.utcnow() is deprecated and scheduled for removal in a future version. Use timezone-aware objects to represent datetimes in UTC: datetime.datetime.now(datetime.UTC).
  dt.utcnow().isoformat()
Out[20]: '2026-02-25T16:09:39.256359'

It could easily be updated to remove that, however I don't think that's a big deal.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think removing deprecation warnings is kind of a big deal, I would go for it.

Going forward, I think we should eventually make tests running with the latest version of everything (deps and Python) fail in case of warnings.

Copy link
Member Author

Choose a reason for hiding this comment

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

I meant that having the additional +00:00 is not a big deal

Comment on lines +131 to +135
dist = None
for ep in entry_points(group='scrapy'):
if ep.name == 'settings':
dist = ep.dist
break
Copy link
Member Author

Choose a reason for hiding this comment

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

Current minimum Python is 3.10, which always includes importlib.metadata

@codecov
Copy link

codecov bot commented Feb 25, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.25%. Comparing base (7aa5711) to head (11412f8).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #104      +/-   ##
==========================================
+ Coverage   88.65%   89.25%   +0.59%     
==========================================
  Files          14       14              
  Lines         873      856      -17     
==========================================
- Hits          774      764      -10     
+ Misses         99       92       -7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment on lines +21 to +22
super().add_options(parser)
parser.add_argument(
Copy link
Member Author

Choose a reason for hiding this comment

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

Current minimum Scrapy is 2.7, which always uses argparse.

import warnings
from contextlib import contextmanager
from importlib.metadata import PathDistribution
from datetime import timezone, datetime as dt
Copy link
Contributor

Choose a reason for hiding this comment

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

I read somewhere, cannot find it now, a recommendation to import datetime as dt and datetime.datetime as datetime (or use dt.datetime, which would avoid confusion). What do you think?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah I think I read the same thing and just implemented it wrong, import datetime as dt makes for better namespacing with dt.datetime 😄

Copy link
Contributor

@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.

Some of these changes could have been addressed automatically by Ruff’s UP rules. Maybe we should do something like scrapinghub/spidermon#468 in this repository soon in the future.

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.

2 participants