Skip to content

Comments

Fix tests and update headers#175

Merged
sdfordham merged 3 commits intoAnthonyBloomer:masterfrom
groegercesg:fix_tests_and_update_headers
Nov 4, 2025
Merged

Fix tests and update headers#175
sdfordham merged 3 commits intoAnthonyBloomer:masterfrom
groegercesg:fix_tests_and_update_headers

Conversation

@groegercesg
Copy link
Contributor

@groegercesg groegercesg commented Nov 1, 2025

First of all, thanks for the excellent Repo - really useful!

I used it around a year ago with great success, but when I pulled it recently, I was consistently getting 403 Forbidden. After some digging I realised this was Daft blocking requests based on the UserAgent. In this PR, I'm introducing two things:

  • Rewriting the tests to use fixtures, this is so they're no longer calling out to the live site - which isn't ideal if the site goes down, starts blocking requests (like now) or changes something about the API
  • Adding the ability for users to override the headers, this means that they can better react to API changes from daft in future. I left the older headers present, as I don't think it's wise for us to start shipping something that will constantly change.
    • Instead users can use something like fake-useragent or bring their own

Development

I ran black (black .) and the unit tests (python -m unittest discover -s tests), let me know if I should do something else to comply with the coding standards of this package.

User Agent Testing

As a side note for anyone interested, the default user agent "" seems to basically be blocked 100% of the time - resulting an a 403 Forbidden. I did a quick bit of testing with fake-useragent, and saw much better success rates using more recent ones:

Of 100 Random User Agent, Pass Rate was: 61.0%
---------------
Number Succeeded: 61
Number Failed: 39

Here's a quick gist of how I did the test: https://gist.github.com/groegercesg/f31113d9417c174bcd3c297e7a6fd282

Feedback wanted

  1. I'm wondering if we should start shipping daftlistings with something like fake-useragent, and we can try a couple user agents (up to some sensible limit) to make the request be accepted
  2. I'm open to removing the first commit, if we want to actually just tests against the live site, we'll have to then update them with a UserAgent though (and then likely need to update them in future etc)

- add a new fixture for multiple studio apartments, pulled direct from daft
- rewrite tests that previously called out to the live website, to use fixtures
"platform": "web",
}

def set_headers(self, headers: dict):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, I can make dict -> Dict[str, str] to have better typing

@sdfordham
Copy link
Collaborator

Yes mocking the endpoint is better and then have some kind of "integration test" that checks the POST. Can you check please your gist it just gives me 100% fail? I think there is an issue with the code, e.g. user_agent doesn't appear to be a valid argument to initialise the class in your fork.

@groegercesg
Copy link
Contributor Author

Ah whoops sorry, that was on older one from a previous impl. I’ve updated the gist (https://gist.github.com/groegercesg/f31113d9417c174bcd3c297e7a6fd282) - that should work now.

If I had to guess we will get slight different pass rates, as I don’t think the random useragent command allows a custom seed - so it’ll be different user agents used.

@sdfordham
Copy link
Collaborator

Looking at the failing values for user-agent strings, it seems they are all a version of Chrome <130. Regardless, it would be a bad idea to make setting the user-agent as a requirement for using the package as it adds needless complexity for someone wanting to just get some results from the website. It would be better to pick one we know works and is not super exotic and use that. Probably it is good to keep the set_headers method however in case one does want to use it.

Can you add a commit that puts in a default user agent string please, then we should be good to go 👍

@groegercesg groegercesg force-pushed the fix_tests_and_update_headers branch from 89c5b4f to a3be80c Compare November 2, 2025 16:07
@groegercesg groegercesg force-pushed the fix_tests_and_update_headers branch from a3be80c to 20dce0f Compare November 2, 2025 16:13
@groegercesg
Copy link
Contributor Author

Hmm, yeah makes sense to not force users to pick one.

I've added a commit with a new default user agent that works, and I've updated the unit tests as well.
For reference, I picked it using:

from fake_useragent import UserAgent
ua = UserAgent(os='Windows', platforms='desktop', min_version=130.0)

print(ua.chrome)

@sdfordham sdfordham self-assigned this Nov 4, 2025
@sdfordham sdfordham self-requested a review November 4, 2025 20:24
@sdfordham sdfordham merged commit 3e711f4 into AnthonyBloomer:master Nov 4, 2025
2 checks passed
@sdfordham
Copy link
Collaborator

Thank you @groegercesg this is a very useful addition to the package 🔥

@groegercesg groegercesg deleted the fix_tests_and_update_headers branch November 8, 2025 23:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants