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

[ API ] fix API errors #163

Closed
wants to merge 13 commits into from

Conversation

krishsharma0413
Copy link

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 is 2 it is giving an error

image

second, with pytest out of 43 total tests, 9 failed, 34 passed but when i changed the tag:3d to say tag: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} becomes payload = 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...

@krishsharma0413
Copy link
Author

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 requests.get will not get 200, ever

@krishsharma0413
Copy link
Author

7 failed, 37 passed

FAILED tests/test_cli.py::TestHentaiCLI::test_cli_download - FileNotFoundError: [WinError 2] The system cannot find the...
FAILED tests/test_cli.py::TestHentaiCLI::test_cli_log - FileNotFoundError: [WinError 2] The system cannot find the file...
FAILED tests/test_cli.py::TestHentaiCLI::test_cli_preview - FileNotFoundError: [WinError 2] The system cannot find the ...
FAILED tests/test_hentai.py::TestHentai::test_artist - AssertionError: Lists differ: [Tag([31 chars]hindol', url='https...
FAILED tests/test_hentai.py::TestHentai::test_category - AssertionError: Lists differ: [Tag([34 chars]anga', url='https...
FAILED tests/test_hentai.py::TestHentai::test_language - AssertionError: Lists differ: [Tag([89 chars]ount=69378), Tag(...
FAILED tests/test_hentai.py::TestHentai::test_tag - AssertionError: Lists differ: [Tag(id=19018, type='tag', name='dark...

progress 🎉

@krishsharma0413
Copy link
Author

Oh the test should have passed since the testing data is old Tag(i[29 chars]shindol', url='https://nhentai.net/artist/shindol/', count=279) | Tag(i[29 chars]shindol', url='https://nhentai.net/artist/shindol/', count=291)

though i still have no idea about test_cli since i have never worked with shlex

@hentai-chan
Copy link
Owner

hentai-chan commented Sep 14, 2022

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 is 2 it is giving an error

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.

As a test won't work since requests.get will not get 200, ever

Make corrections to the code as you see fit at the moment, we can discuss your changes during the code review later.

@krishsharma0413
Copy link
Author

if (len(query)<3):
    raise ArgumentError("todo: add descriptive error message here")

Alright i will look into that as well

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.

I found 3dlive to be working good enough

@hentai-chan
Copy link
Owner

A note on branching policy: contributors should always target the development branch rec-hentai, not master. We do this to make some final tests in order to ensure that the next release works as intended.

@krishsharma0413
Copy link
Author

oh, i was looking for dev-api branch but couldn't find one so i just targeted master.

@krishsharma0413 krishsharma0413 changed the base branch from master to rec-hentai September 14, 2022 19:18
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:
Copy link
Owner

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

Copy link
Author

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

Copy link
Owner

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.

@krishsharma0413
Copy link
Author

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 tag:! shows error but tag:@ doesn't

Do you know what is going on?

@hentai-chan
Copy link
Owner

Do you know what is going on?

Just a wild guess but try to update the 177013.json test reference file in the tests directory. On your branch, open a terminal session, import the hentai module and run

> from hentai import Hentai, Option
> doujin = Hentai(177013)
> doujin.export(filename=Path("tests").joinpath("f{doujin.id}.json"), options=[Option.Raw])

@krishsharma0413
Copy link
Author

4 failed, 40 passed in 91.19s (0:01:31)

4 failed are tests/test_cli.py::TestHentaiCLI::test_cli_download tests/test_cli.py::TestHentaiCLI::test_cli_log tests/test_cli.py::TestHentaiCLI::test_cli_preview tests/test_hentai.py::TestHentai::test_num_favorites_edge_case of which i have no idea

@krishsharma0413
Copy link
Author

krishsharma0413 commented Sep 14, 2022

also why is json.dump([doujin.json for doujin in iterable], file_handler) storing it as a list?
Thats just gonna make all the tests fail... i had to remove the dict from the list since API doesn't return list

ah nvm understood

@krishsharma0413
Copy link
Author

krishsharma0413 commented Sep 14, 2022

3 failed, 41 passed in 86.30s (0:01:26)

tests/test_cli.py::TestHentaiCLI::test_cli_download tests/test_cli.py::TestHentaiCLI::test_cli_log tests/test_cli.py::TestHentaiCLI::test_cli_preview failed

with that i think that's all i can do, i need some code review and suggestions for test_cli.py by you now

@hentai-chan
Copy link
Owner

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

@krishsharma0413
Copy link
Author

I checked everything and couldn't find anything else to improve

@krishsharma0413 krishsharma0413 marked this pull request as ready for review September 16, 2022 13:17
@krishsharma0413 krishsharma0413 changed the title [ API ] fix API errors WIP [ API ] fix API errors Sep 16, 2022
@krishsharma0413
Copy link
Author

today i will get code review?

@hentai-chan
Copy link
Owner

today i will get code review?

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 rec-hentai branch to ensure everything works perfectly, though this in and of itself doesn't take much time.

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 (3.2.10) will prove to be more difficult (but not impossible) in case nhentai does decide to implement OAuth2 authentication. Perhaps that's just a bullet I have to bite in the moment.

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.

@hentai-chan hentai-chan self-assigned this Sep 17, 2022
@hentai-chan hentai-chan added this to Backlog in Global Task Tracker via automation Sep 17, 2022
@hentai-chan hentai-chan added the triage-required Review required label Sep 17, 2022
@krishsharma0413
Copy link
Author

Hit me up on discord resetxd#8278

@krishsharma0413
Copy link
Author

about this method browse_homepage, do we really need a set as a return type and make our work harder? won't list be a better option here?

    @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

Global Task Tracker automation moved this from Backlog to In Progress Sep 25, 2022
Copy link
Owner

@hentai-chan hentai-chan left a 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.

src/hentai/hentai.py Show resolved Hide resolved
src/hentai/hentai.py Outdated Show resolved Hide resolved
src/hentai/hentai.py Outdated Show resolved Hide resolved
src/hentai/hentai.py Show resolved Hide resolved
src/hentai/hentai.py Outdated Show resolved Hide resolved
src/hentai/hentai.py Outdated Show resolved Hide resolved
src/hentai/hentai.py Outdated Show resolved Hide resolved
tests/test_utils.py Outdated Show resolved Hide resolved
src/hentai/hentai.py Outdated Show resolved Hide resolved
@hentai-chan
Copy link
Owner

about this method browse_homepage, do we really need a set as a return type and make our work harder? won't list be a better option here?

Believe it or not, the API used to return a list of Hentai objects in the past. I changed it to a set for the following reasons:

  • a set is faster if you care about querying the data structure for checking existence of other objects
  • sets also only contain unique values which was important to me to emphasize here
  • lists are subscriptable and sortable (as opposed to sets though sets are sorted by insertion order, so we still have a correctly sorted collection by virtue of the set initialization)

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 list() method, so I am forcing the end user to make a conscious decision in that regard: do you need access by index? If so convert it to list, else reap the benefits of the set data structure

@krishsharma0413
Copy link
Author

@hentai-chan
== 44 passed in 171.84s (0:02:51) ==

all the test passes, (maybe the CLI test failed because i was having py2 and py3 in the same environment(?) )
This commit has all the requested changes though i might have overlooked some(?)

@krishsharma0413
Copy link
Author

Hey hentai-chan,
Are we still working on this update? This updated was required for this library to function properly once again.
Sorry i had gotten myself into other IRL stuff and wasn't able to update or work on this PR at all. Is there something i missed and need work on again?

I am really sorry for such a delayed reponse.

regards,
resetxd

@hentai-chan
Copy link
Owner

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:

https://community.cloudflare.com/t/prevent-unwanted-visitors-from-using-google-translate-to-access-your-site/197923

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.

@krishsharma0413
Copy link
Author

ah that seems like a good decision!
if they can disable this as well then there really isn't something we can do (at least in our knowledge).

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 close the PR yourself after you have something to say about this.

@hentai-chan
Copy link
Owner

You may continue your work in this sibling repository as a temporary workaround:

AlexandreSenpai/Enma#43

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
triage-required Review required
Projects
Development

Successfully merging this pull request may close these issues.

Library not working due to poor request Module do not work anymore
2 participants