Skip to content

Python Pip `requirements.txt` - Support multiple text files with `requirements` in the name

Background

In !166633 (merged), we introduced a new config class PythonPip that contains the logic to parse requirements.txt. It currently does not contain logic to support nested requirements files that are specified with:

-r other_requirements.txt

or

--requirement other_requirements.txt

The Repository X-Ray Go repo appears to support the first version. However, it is not easily supported by the new in-monolith service introduced in #476177 (closed). This is due to several factors including:

  • Ai::Context::Dependencies::ConfigFileParser currently only supports one file path per config file class.
  • There must be more consideration as to how deeply the files can be nested. We also need precautions against circular dependencies.
  • Fetching additional blobs would require more calls to Gitaly, which may be costly if we want to support nested files recursively.
  • We may have to refactor some parts of the new service to accommodate for special cases like these.

Discussion

With the above complications, I don't believe this support should be a blocker for rolling out the new in-monolith feature in %17.5 (#483928 (closed)).

Since there is currently low usage of Repository X-ray (as of %17.5), it's unlikely that this excluded coverage would be noticeable. I believe we can de-prioritize this support and re-consider it in the next iteration.

Alternatively, instead of supporting nested files, we could consider a simpler approach:

  1. Expand our file glob to match any text file with the word requirements in the name. And,
  2. Update ConfigFileParser to support more than one file per config file class (if applicable) and merge the parsed dependencies across all files.

This alternative is preferred, provided we can rely on the likelihood that almost all requirement files (including nested ones) contain the word requirements. Note that we actually already rely on this fact for parsing the primary/parent config file, requirements.txt. This name is not technically enforced by Pip; it's just the convention.

Proposal

Based on the discussion in #491800 (comment 2122573173), we will move forward with the alternative proposal above. Specifically, we will parse and merge the dependencies from any text file with requirements in the name (and which are placed within two folders deep.)

Edited by Leaminn Ma