Fix fetching bug for requirements.in files

[stulle123]

Hi there,

There seems to be a bug in the file_fetcher.rb class when trying to fetch requirements.in files:

If there's also a requirements.txt file in the repository, the download works.

If not, Dependabot doesn't continue to look for requirements.in files and raises a DependencyFileNotFound exception here.

When I add requirements_in_files.any? to the list of checks the download works just fine. Maybe, there's a better fix for this, didn't investigate the issue any further.

Thanks.

[stulle123]

Regarding the 2nd commit, would it be possible to increase the file size of the *.in and *.txt files to 300 kilobytes?

Especially in large projects, compiled requirement.txt files can get pretty big. Thanks!

[jeffwidman]

👋 Hey @stulle123 thanks for the PR!

A few quick questions:

1. Can you explain the scenario where you've committed a requirements.in file but not a requirements.txt file? I personally always considered it a best practice to commit both... that's kinda the point of generating a fully pinned requirements.txt file. But if there's a valid scenario to commit only the *.in file, then your change seems reasonable to me.
2. The file size limit is because for python we pull in any .txt file, and and only then we try to figure out if it’s a requirements file, because they can have any random name. So we wanted to minimize pulling in large docs files etc...
   Are you actually hitting the 200KB limit in your jobs? I'd be curious to see the repo if it's public--do you have a Job ID?
   If you are hitting the limit, then yes, we should raise it, but it'd be best to do that in a separate PR as the two issues here are orthogonal.
3. Both PR's need tests. If you need help figuring out how to write them give a shout and I'll try to help.

BTW, I noticed this is your first PR on GitHub, so congrats! 🥳 I still remember the first time I opened a PR for an open-source project, a little scary, but I was thrilled when it was merged.

[dws]

It would be even better if the file size heuristic could be configurable, so that it would be possible to override the default size cap when it's not appropriate.

[deivid-rodriguez]

I was thinking that if we're going to make this configurable, it makes more sense to make the name of the manifest configurable, so that we don't have to make any guesses, fetch files that we don't need at all, or put any size limits on what we fetch.

We'd probably need to keep the current logic for backwards compatibility, though.

[stulle123]

Hey @jeffwidman thanks for the quick reply and warm welcome!

Regarding your questions:

1. I have both requirements.in and requirements.txt files in the repo. Just in the case of an error (e.g., file size of requirements.txt is too big ;-)) it makes sense to "fall-back" to the requirements.in file. IMO, that additional check doesn't hurt much.
2. Yeah, I'm hitting the limit of the 200KB (that was the reason why I came across the issue above in the first place). Having this configurable as @dws mentioned would be ideal. For now, I can split up the PR, no problem.
3. Regarding unit tests, can you point to some examples / documentation?

Cheers

[stulle123]

Hey @jeffwidman!

I've just split up this PR into two.

That's the one changing the file size of requirements.txt files to 500 kilobytes --> #5596.

Can you please help me on how to add unit tests for the two changes?

Thanks!

[jeffwidman]

Hey @stulle123 thanks for splitting them out here.

For this one, the test should be relatively straightforward. Look in this file: https://github.com/dependabot/dependabot-core/blob/main/python/spec/dependabot/python/file_fetcher_spec.rb

I'd grep for "with only" and you'll see how the tests were written for other file types. Just need to add one saying "with only a requirements.in".

You can run the tests natively using these instructions, but the easier way to avoid having to muck around with installing the proper ruby libraries locally is to use the docker image. Instructions for booting the image are here, and a few paragraphs in there's a section on running tests within the docker image.

[stulle123]

Hey @jeffwidman just added the unit test! Cheers!

[jeffwidman]

Looks great, thank you!

[jeffwidman]

@stulle123 sorry to bother you, but looks like this code is failing a lint error about line too long, probably because we tidied up our Rubocop settings in #5588.

Do you mind fixing? Should be straightforward.

Also, while you're in there, can you squash to a single commit? that way the test + the change are located in the same commit for anyone in the future doing debugging via git blame.

[stulle123]

Hey @jeffwidman Rubocop checks are green now. As you can see I had some troubles with the commit squashing, but I somehow managed.

[jeffwidman]

Thanks!

I tweaked it slightly so that it doesn't have two return values,
as that can start to imply there's two groups of things, rather
than that they're all on an equal footing.