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

Add a client-side alternative to echoData #15

Open
Gallaecio opened this issue May 18, 2022 · 5 comments
Open

Add a client-side alternative to echoData #15

Gallaecio opened this issue May 18, 2022 · 5 comments
Labels
enhancement New feature or request

Comments

@Gallaecio
Copy link
Contributor

Gallaecio commented May 18, 2022

While working on documentation for https://docs.zyte.com/ about setting request metadata, I am starting to think that maybe we should not send echoData to the server, and instead keep track of it on the client side.

I get the usefulness of echoData for generic HTTP clients that have no request metadata tracking feature, but a client with that feature, or one made specifically for Zyte Data API, should probably keep track of request metadata on the client side. In fact, maybe https://github.com/scrapy-plugins/scrapy-zyte-api should discourage its use altogether, in favor of Request.meta.

This is of course based solely on the current implementation of Zyte Data API. It is possible that future features will make it worthwhile to include echoData (e.g. some web UI that allows to keep track of your requests, where getting access to echoData would be useful).

Thoughts?

@Gallaecio Gallaecio added the question Further information is requested label May 18, 2022
@kmike
Copy link
Collaborator

kmike commented May 18, 2022

This makes sense to me, in general.
It seems echoData is not used in scrapy-zyte-api; are you thinking about documentation changes?
Here echoData is used in the CLI tool, so that the output contains source URL by default. What alternative do you see?

@lopuhin
Copy link

lopuhin commented May 18, 2022

maybe we should not send echoData to the server, and instead keep track of it on the client side.

If it's still called echoData in the client, then it can be surprising if it's not sent to the server IMO for someone aware of the HTTP API, e.g. if you're doing some debugging.

@Gallaecio
Copy link
Contributor Author

Gallaecio commented May 19, 2022

It seems echoData is not used in scrapy-zyte-api; are you thinking about documentation changes?

Yes. In the docs.zyte.com content I am preparing about echoData, in the code example tab set, we could cover how to use echoData in Scrapy but explain how Request.meta may be a better choice. I would also remove echoData from the example in https://github.com/scrapy-plugins/scrapy-zyte-api/blob/main/README.rst#how-to-use.

If it's still called echoData in the client, then it can be surprising if it's not sent to the server IMO for someone aware of the HTTP API, e.g. if you're doing some debugging.

Yes, that is a very good point.

For python-zyte-api, maybe we should leave things as they are for now, but in the future, if there is demand for it, add an AsyncClient parameter and command-line interface option to track echoData on the client side. I don’t expect demand for it any time soon, but it may be a good option for people hitting the API request length limit because of their echoData content.

@kmike
Copy link
Collaborator

kmike commented May 21, 2022

I wonder if it'd be better to have this feature, but not have it tied to echoData. Or, alternatively, make echoData handling optional.

@Gallaecio
Copy link
Contributor Author

Gallaecio commented May 24, 2022

I wonder if it'd be better to have this feature, but not have it tied to echoData.

Makes a lot of sense. In the scenario where echoData causes the API request to be too long, users could move some data from echoData to a client-specific field (e.g. clientData, requestMetadata), and still keep some data in echoData (in case there is a benefit in the future to have data there beyond getting it back in the client).

@Gallaecio Gallaecio added enhancement New feature or request and removed question Further information is requested labels May 24, 2022
@Gallaecio Gallaecio changed the title Do we need to include echoData in API requests? Add a client-side alternative to echoData May 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants