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

Request type could be improved #266

Open
metalwarrior665 opened this issue Aug 5, 2022 · 1 comment
Open

Request type could be improved #266

metalwarrior665 opened this issue Aug 5, 2022 · 1 comment
Assignees
Labels
t-tooling Issues with this label are in the ownership of the tooling team.

Comments

@metalwarrior665
Copy link
Member

  1. A lot of the properties are | undefined. Why? Is there a case where the queue would not have those properties? I think we would need 2 different types, one for request options which have mostly optional values but when you use request as a type you get from somewhere, all these will be populated by default values so the type should reflect that.
  2. handledAt should be string | null as null is a value for in progress
@gippy gippy added the t-platform Issues with this label are in the ownership of the platform team. label Oct 31, 2022
@fnesveda fnesveda removed the t-platform Issues with this label are in the ownership of the platform team. label Nov 2, 2022
@mtrunkat mtrunkat added t-platform Issues with this label are in the ownership of the platform team. t-tooling Issues with this label are in the ownership of the tooling team. and removed t-platform Issues with this label are in the ownership of the platform team. labels Mar 8, 2023
@B4nan
Copy link
Member

B4nan commented Mar 20, 2023

A lot of the properties are | undefined.

Can you list which ones? I can see loadedUrl, id, and headers, rest seems to be already mandatory. And I can already see why - the object represents a request, not necessarily processed - you don't know the loadedUrl or headers until you process it, I guess same applies for the id.

https://github.com/apify/crawlee/blob/master/packages/core/src/request.ts#L72

FYI we already have two types, one is the Request class itself, and one is the RequestOptions interface.

So we should maybe have a third interface for processed request, and use that when appropriate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
t-tooling Issues with this label are in the ownership of the tooling team.
Projects
None yet
Development

No branches or pull requests

6 participants