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

Updated duckduckgo-search. (Fixes #490) #506

Closed
wants to merge 1 commit into from

Conversation

CameronJGrant
Copy link

I've updated everything to the latest versions and added poetry as a dependency management system. Note that later versions of "duckduckgo-search" need to use "curl-cffi" and that brought about a host of new issues that needed to be dealt with. That's why there are a so many edits to the Dockerfile.

@Jipok
Copy link

Jipok commented Dec 22, 2023

Why remove requirements.txt?

@CameronJGrant
Copy link
Author

It hasn't been removed. It's been replaced with the "pyproject.toml" file (Poetry https://python-poetry.org/).

It works the same except it ensures package versions work together. It also generates a lock file, with hashes for each package, so that your environment is identical to everyone elses. This is especially important when building Dockerfiles. Otherwise we can have a situation where "chatgpt-telegram-bot" works on your system, but not on mine.

If you're not familiar with it, check it out. It will help with future projects.

@Jipok
Copy link

Jipok commented Dec 24, 2023

It hasn't been removed.

It was clearly deleted judging by what Github shows.

It works the same except...

except that it requires installing poetry. In which I don’t see any point at all for such projects.
From my point of view, you are adding an additional dependency that needs to be installed on the system in order to have a slightly more convenient way of describing dependencies. I really don’t like this kind of stuff, it’s overcomplicated. I don't understand why you're bringing Docker into this at all.

@k3it
Copy link
Contributor

k3it commented Dec 24, 2023

@Jipok has a point. it looks like poetry repo has 612 open issues and 85 pull requests, so it's a highly complex dependency all by itself.

@CameronJGrant
Copy link
Author

CameronJGrant commented Dec 24, 2023

I've editted this response.

I agree with both of your points. This should have been maybe 3 separate PR's. One for the fix. One for the overall updates. And one for the dependency manager. I'll edit this PR when I have time. Otherwise we can take @Jipok's PR and add a Dockerfile fix to it and it will be fine too.

@CameronJGrant CameronJGrant reopened this Dec 24, 2023
@CameronJGrant
Copy link
Author

I've made this simpler now. No more Poetry.

@k3it
Copy link
Contributor

k3it commented Dec 24, 2023

quick tested and the build works with the #501 (#504 also), looks good to me. thank you!

@CameronJGrant CameronJGrant changed the title Changed dependency management to poetry and updated everything. (Fixes #490) Updated duckduckgo-search. (Fixes #490) Dec 26, 2023
@k3it
Copy link
Contributor

k3it commented Jan 28, 2024

paging @n3d1117

@tobias-strenger
Copy link

tobias-strenger commented Feb 21, 2024

For me, this literally doubled the image size. Ie Dive shows a lot in /usr/lib/. I don't know exactly how to selectively remove the build dependencies.

@CameronJGrant
Copy link
Author

For me, this literally doubled the image size. Ie Dive shows a lot in /usr/lib/. I don't know exactly how to selectively remove the build dependencies.

Hmm. I did remove the gcc build dependencies in the line RUN apk del build-base. There shouldn't be any other build dependencies. The "curl-impersonate" libraries are copied from a separate image so I don't build them.

It may be a moot point anyway as I've heard nothing from @n3d1117.

Copy link

@A1RWALK3R A1RWALK3R left a comment

Choose a reason for hiding this comment

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

For now it is

duckduckgo_search~=5.0
curl-cffi~=0.6.3b1

@n3d1117
Copy link
Owner

n3d1117 commented Mar 11, 2024

Hi all, sorry for the long wait, are these changes still relevant? Or is it enough to update to latest duckduckgo_search and curl-cffi like @A1RWALK3R suggested?

@CameronJGrant
Copy link
Author

Thanks to @yifeikong for his update of curl_cffi @A1RWALK3R suggestion should do it.

@chk86
Copy link

chk86 commented May 11, 2024

This fix no longer works. DDG is returning "Failed to get response...202 Ratelimit".

@chk86
Copy link

chk86 commented May 11, 2024

And by this fix I mean @A1RWALK3R 's suggestion.

@A1RWALK3R
Copy link

@chk86 I think you can try duckduckgo_search==5.3.0b4

I haven't checked it yet, but I found useful information about 202 Ratelimit here

@chk86
Copy link

chk86 commented May 12, 2024

Thanks, @A1RWALK3R . I saw that actually, that fix doesn't work, either. I tried recreating the bot with that version. I'll just be patient till they get it sorted.

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.

7 participants