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

remove_namespaces asserts on json TextResponse #295

Open
mohmad-null opened this issue May 3, 2024 · 5 comments
Open

remove_namespaces asserts on json TextResponse #295

mohmad-null opened this issue May 3, 2024 · 5 comments

Comments

@mohmad-null
Copy link

Regression.
As part of my unit tests, I have a well-formed JSON file that I'm using in some tests as "bad" data (the script wants XML).

I do something like this:

response = TextResponse(body='path-to.json', encoding='utf8')
response.selector.remove_namespaces()

This used to work (1.7.x), now (2.11.1) it asserts:

    def remove_namespaces(self) -> None:
        """
        Remove all namespaces, allowing to traverse the document using
        namespace-less xpaths. See :ref:`removing-namespaces`.
        """
		for el in self.root.iter("*"):
		AttributeError: 'dict' object has no attribute 'iter'

I appreciate JSON doesn't have namespaces, but it should fail gracefully, not raise an assertion.

@wRAR
Copy link
Member

wRAR commented May 3, 2024

It's parsel, not Scrapy.

@mohmad-null
Copy link
Author

Yes, I noticed that, but I figured Scrapy should catch it.

@wRAR
Copy link
Member

wRAR commented May 4, 2024

I don't think Scrapy should catch it, and Scrapy will only able to do that by overriding remove_namespaces().

@mohmad-null
Copy link
Author

But AttributeError: 'dict' object has no attribute 'iter' is a meaningless error for a scrapy user.
If there's an exception (fine), it should be something meaningful "Cannot remove namespaces from JSON" or something - that's why I consider it a Scrapy issue.

@wRAR
Copy link
Member

wRAR commented May 4, 2024

The code is in parsel, not Scrapy. It should be done in parsel, not in Scrapy. Migrating.

@wRAR wRAR transferred this issue from scrapy/scrapy May 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants