Remove `request.id`

[Pijukatel]

As of now, request.id is an implementation detail of the Apify-based request queue. There is no need to have it in Crawlee, and it can be fully replaced by request.unique_key

[Pijukatel]

These 3 branches are an experiment of removing the request.id completely. All of them would have to be merged:
In this order:
https://github.com/apify/apify-client-python/tree/remove-request-id
https://github.com/apify/crawlee-python/tree/remove-request-id
https://github.com/apify/apify-sdk-python/tree/remove-request-id

[janbuchar]

As of now, request.id is an implementation detail of the Apify-based request queue. There is no need to have it in Crawlee, and it can be fully replaced by request.unique_key

🤯 I like that simplification! How about other possible storage clients? Will they be expected to maintain a mapping of unique key -> request id internally if they need it? I imagine it might happen for many of them - for filesystem storage client, you'll need some "safe" file names, in an SQL storage client, using a TEXT column as primary key may be suboptimal in some DBMSs, and so on...

https://github.com/apify/apify-client-python/tree/remove-request-id

Do we really need to add this to the API client? It feels kinda redundant there, can't we handle the unique_key -> id conversion in the ApifyRequestQueueStorageClient in the SDK?

[Pijukatel]

I thought about those. I think that having a direct deterministic transformation function is better (simpler and no need to maintain mapping) than having to wait for the client to give us the id and then keep the mapping between unique_key and id.

Let's look at them case by case:

• In the case of the Apify platform and the client, we know the transformation function, so we can use it to our benefit. It is defined in the Apify specific client as something internal, so that seems to me like a correct place for it.
• In the case of file system storage client, I created another deterministic transformation function, so no need for id.
• In the case memory storage client, it does not even need anything like id, it uses a unique key directly.
• In the potential case of future clients, I would leave it up to them to either define the transformation function, or use ProcessedRequest.id and use that for some sort of unique_key -> id mapping.

Do we really need to add this to the API client? It feels kinda redundant there, can't we handle the unique_key -> id conversion in the ApifyRequestQueueStorageClient in the SDK?

Yes, there is a redundancy, but I would say that the unique_key-based methods are more natural as the id is something internal to the storage. I would maybe go even as far as making the ID-based methods internal in the next big release.

[janbuchar]

I thought about those. I think that having a direct deterministic transformation function is better (simpler and no need to maintain mapping) than having to wait for the client to give us the id and then keep the mapping between unique_key and id.

Let's look at them case by case:

You raise a good point that there is no good reason to rely on some autoincremented value if we can already have a transformation from unique key to a shorter string. Can you point me to the implementation of this in the filesystem storage client?

Do we really need to add this to the API client? It feels kinda redundant there, can't we handle the unique_key -> id conversion in the ApifyRequestQueueStorageClient in the SDK?

Yes, there is a redundancy, but I would say that the unique_key-based methods are more natural as the id is something internal to the storage. I would maybe go even as far as making the ID-based methods internal in the next big release.

I could get behind that, but the client is supposed to match the Apify API as closely as possible. I don't think we should wrap it in ways that change how it's used, if that makes any sense.

[vdusek]

Can you point me to the implementation of this in the filesystem storage client?

Maybe here -

crawlee-python/src/crawlee/storage_clients/_file_system/_request_queue_client.py

Line 816 in 1d6a6fe

def _get_file_base_name_from_unique_key(unique_key: str) -> str:

.

[Pijukatel]

Do we really need to add this to the API client? It feels kinda redundant there, can't we handle the unique_key -> id conversion in the ApifyRequestQueueStorageClient in the SDK?

Yes, there is a redundancy, but I would say that the unique_key-based methods are more natural as the id is something internal to the storage. I would maybe go even as far as making the ID-based methods internal in the next big release.

I could get behind that, but the client is supposed to match the Apify API as closely as possible. I don't think we should wrap it in ways that change how it's used, if that makes any sense.

I am also fine with not doing apify-client-python change and keep the transformation in apify-sdk-python as internals of ApifyRequestQueueClient