-
-
Notifications
You must be signed in to change notification settings - Fork 19
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
[ API ] fix API errors #163
Conversation
def test_random_hentai(self):
random_hentai = Utils.get_random_hentai()
response = requests.get(random_hentai.url)
self.assertEqual(response.status_code, 200, msg=f"Failing URL: {response.url} (status code: {response.status_code})") As a test won't work since |
progress 🎉 |
Oh the test should have passed since the testing data is old though i still have no idea about |
If this is true (which I will check later during the weekend when I have time) you should consider adding a check in any methods that use this parameter in this fashion: if (len(query)<3):
raise ArgumentError("todo: add descriptive error message here") Change the sample query in the unit tests to something else, but try to look for a tag that yields few results to decrease server load since unit tests are run fairly frequently during development.
Make corrections to the code as you see fit at the moment, we can discuss your changes during the code review later. |
Alright i will look into that as well
I found |
A note on branching policy: contributors should always target the development branch |
oh, i was looking for |
src/hentai/hentai.py
Outdated
payload = {'query': query, 'page': 1, 'sort': sort.value} | ||
with handler.get(urljoin(Hentai.HOME, '/api/galleries/search'), params=payload) as response: | ||
payload = f"?query={query}&page=1&sort={sort.value}" | ||
with handler.get(urljoin(Hentai.HOME, '/api/galleries/search'+payload)) as response: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
strings should never be concatenated, see also: https://pakstech.com/blog/python-build-urls/ for an example of how to build a URL correctly, if you see yourself needing an auxiliary method in hentai.py
, consider implementing a __build_api
in the RequestHandler
class
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh its alright, its just a temporary solution, later i plan on using urllib.urlencode
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
alright I will let you do your thing until you request a code review, in the meantime I will be busy working on other projects, but you can still ask me questions (if you have any). I usually respond in a reasonable timeframe.
On checking the Tag error again, i can conclude that it isnt about len but it is something completely different that i don't seem to understand, for example Do you know what is going on? |
Just a wild guess but try to update the > from hentai import Hentai, Option
> doujin = Hentai(177013)
> doujin.export(filename=Path("tests").joinpath("f{doujin.id}.json"), options=[Option.Raw]) |
4 failed are |
ah nvm understood |
with that i think that's all i can do, i need some code review and suggestions for test_cli.py by you now |
sure, I will let you now when I get to it, seeing that I am currently busy with my physics studies your code review will have to wait for a few days, which shall give you ample time to improve the quality of the code in the meantime |
I checked everything and couldn't find anything else to improve |
I will make the review sometime around Sunday. There are a few things that have been on my mind for a quite a while now that I may want to test out myself (for which I would open up a separate branch), and even in the best-case scenario I would still have to update the docs on my website and stress-test the I am a little hesitant whether I will consider your patch a permanent addition to the library code or a temporary fix until nhentai devs work out a proper solution for the REST API (see also: #155). We don't know for certain how long your workaround is going to stay functional because it's quite a hack, to be honest. So, in the event I decide to add new features to this library reverting back the current version ( But, depending on your reliability, there are a few things I could use your help with in the future which is something we could discuss in private another time on Discord if you are interested, though these only involve long term plans with the library and the direction it's heading to. |
Hit me up on discord |
about this method @staticmethod
def browse_homepage(start_page: int, end_page: int, handler: RequestHandler=RequestHandler(), progressbar: bool=False) -> Set[Hentai]:
"""
Return a list of `Hentai` objects that are currently featured on the homepage
in range of `[start_page, end_page]`. Each page contains as much as 25 results.
Enable `progressbar` for status feedback in terminal applications.
"""
if start_page > end_page:
raise ValueError(f"{COLORS['red']}{os.strerror(errno.EINVAL)}: start_page={start_page} <= {end_page}=end_page is False{COLORS['reset']}")
data = set()
for page in tqdm(**_progressbar_options(range(start_page, end_page+1), 'Browse', 'page', disable=progressbar)):
with handler.get(urljoin(Hentai.HOME, 'api/galleries/all' + "?" + urlencode({'page': page})) ) as response:
for raw_json in response.json()['result']:
data.add(Hentai(json=raw_json))
return data changes to @staticmethod
def browse_homepage(start_page: int, end_page: int, handler: RequestHandler=RequestHandler(), progressbar: bool=False) -> Set[Hentai]:
"""
Return a list of `Hentai` objects that are currently featured on the homepage
in range of `[start_page, end_page]`. Each page contains as much as 25 results.
Enable `progressbar` for status feedback in terminal applications.
"""
if start_page > end_page:
raise ValueError(f"{COLORS['red']}{os.strerror(errno.EINVAL)}: start_page={start_page} <= {end_page}=end_page is False{COLORS['reset']}")
data = list()
for page in tqdm(**_progressbar_options(range(start_page, end_page+1), 'Browse', 'page', disable=progressbar)):
with handler.get(urljoin(Hentai.HOME, 'api/galleries/all' + "?" + urlencode({'page': page})) ) as response:
for raw_json in response.json()['result']:
data.append(Hentai(json=raw_json))
return data |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the late review, I am really busy at the moment but I finally got around to review your pull request :) Overall good work! I remarked a few things that needs to be work, don't worry about the CLI related tests I will take care of them once your branch has merged.
Believe it or not, the API used to return a list of
The only "downside" is that the result of this function is not subscriptable which can be easily worked around by converting the result to list with the |
@hentai-chan all the test passes, (maybe the CLI test failed because i was having py2 and py3 in the same environment(?) ) |
Hey hentai-chan, I am really sorry for such a delayed reponse. regards, |
Sorry for the late reply, but I have actually decided to no longer go forward with this solution (which was more of a hack anyway) since there already exists a documented solution for cloudflare-protected sites to disable clients from accessing the site through a Google-translate link: Thus it would only be a matter of time for the web administrator to catch on to this change if they keep an eye open on their API usage, which is not unlikely considering this project and discussions revolving around it are taking place on a public forum. |
ah that seems like a good decision! maybe you have looked for another workaround? well anyways i hope you can look for something to revive the NSFW community up again. |
You may continue your work in this sibling repository as a temporary workaround: I can try one more time to get in touch with the web admin at nhentai in order to find a solution that works for both sides, but past efforts have made me doubt that I will receive a response. |
I am still working on this.
Though i noticed that with tag searching test case
tag:3d
it doesn't seem to work... it isn't the code issue but i think nhentai removed searching for characters less than 3 and since 3d is2
it is giving an errorsecond, with
pytest
out of 43 total tests,9 failed, 34 passed
but when i changed thetag:3d
to saytag:3dlive
then the testcases for tag searching works@hentai-chan thoughts?
currently i am making a temporary fix with changing query parameter into a string and then concatenating the string to make URL so
payload = {'query': query, 'page': page, 'sort': sort.value}
becomespayload = f"?query={query}&page={page}&sort={sort.value}"
this is just a temporary solution and if everything works i will make a proper function to have something like this...