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.

Sign up for GitHub

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

Jump to bottom

feat: add new model for HttpHeaders #544

Merged
merged 8 commits into from
Oct 2, 2024
Merged

feat: add new model for HttpHeaders #544

merged 8 commits into from
Oct 2, 2024

Conversation

vdusek
Copy link
Collaborator

@vdusek vdusek commented Sep 25, 2024 •
edited
Loading

Description

  • Unify the HTTP headers type across the project.
  • I used just the type alias plus custom validator in the Request model to reach the same result (lowercase & sorted).
    • Of course, now it is only in the Request, but I believe it is not a problem.
  • Take it out from fix!: merge payload and data fields of Request #542.

Issues

  • N/A

Testing

  • N/A

Checklist

  • CI passed

@vdusek vdusek added t-tooling Issues with this label are in the ownership of the tooling team. adhoc Ad-hoc unplanned task added during the sprint. debt Code quality improvement or decrease of technical debt. labels Sep 25, 2024
@vdusek vdusek added this to the 99th sprint - Tooling team milestone Sep 25, 2024
@vdusek vdusek self-assigned this Sep 25, 2024
@github-actions github-actions bot added the tested Temporary label used only programatically for some analytics. label Sep 25, 2024
@B4nan B4nan changed the title refactor: unite the HTTP headers type refactor: unify the HTTP headers type Sep 25, 2024
@vdusek vdusek marked this pull request as draft September 27, 2024 17:50
@vdusek vdusek marked this pull request as ready for review October 2, 2024 09:27
janbuchar
janbuchar reviewed Oct 2, 2024
View reviewed changes
Copy link
Collaborator

@janbuchar janbuchar left a comment

Choose a reason for hiding this comment

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

What about http client? That uses HttpHeaders as well and I'd expect normalization to work there.

src/crawlee/_request.py Outdated Show resolved Hide resolved
@vdusek vdusek requested a review from janbuchar October 2, 2024 13:02
janbuchar
janbuchar reviewed Oct 2, 2024
View reviewed changes
src/crawlee/_request.py Show resolved Hide resolved
src/crawlee/_request.py Outdated Show resolved Hide resolved
@vdusek vdusek requested a review from janbuchar October 2, 2024 14:32
@vdusek vdusek changed the title refactor: unify the HTTP headers type feat: add new model for HttpHeaders Oct 2, 2024
janbuchar
janbuchar approved these changes Oct 2, 2024
View reviewed changes
Copy link
Collaborator

@janbuchar janbuchar left a comment

Choose a reason for hiding this comment

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

LGTM

@vdusek vdusek merged commit 854f2c1 into master Oct 2, 2024
20 checks passed
@vdusek vdusek deleted the unite-http-headers-type branch October 2, 2024 14:52
@vdusek vdusek mentioned this pull request Oct 22, 2024
Merged
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@janbuchar janbuchar janbuchar approved these changes

Assignees

@vdusek vdusek

Labels
adhoc Ad-hoc unplanned task added during the sprint. debt Code quality improvement or decrease of technical debt. t-tooling Issues with this label are in the ownership of the tooling team. tested Temporary label used only programatically for some analytics.
Projects
None yet
Milestone
99th sprint - Tooling team
Development

Successfully merging this pull request may close these issues.
2 participants
@vdusek @janbuchar