thoth-station / kebechet

I'm Kebechet bot, goddess of freshness - I will keep your source code fresh and up-to-date
24 stars 20 forks source link

[3pt] Allow local and git package refereces in update manager #1125

Closed codificat closed 1 year ago

codificat commented 2 years ago

Problem statement

When I use Kebechet's update manager, which uses pipenv to resolve dependencies, it refuses to invoke pipenv lock if my Pipfile contains references to local paths or pointers to git.

For example, see https://github.com/AICoE/sefkhet-abwy/issues/192 where Kebechet says:

Kebechet cannot support maintaining this application as it contain's git version of packages.

This is a bit frustrating, because pipenv would actually handle these.

Continuing the example above, the sefkhet-abwy repository's Pipfile contains a git referenced package. But pipenv lock works:

$ pipenv lock
Locking [dev-packages] dependencies...
Building requirements...
Resolving dependencies...
✔ Success! 
Locking [packages] dependencies...
Building requirements...
Resolving dependencies...
✔ Success! 
Updated Pipfile.lock (e1d779)!

$ git status
On branch master
Your branch is up to date with 'upstream/master'.

Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
    modified:   Pipfile.lock

no changes added to commit (use "git add" and/or "git commit -a")

$ pipenv graph | grep -A3 ChatterBot
ChatterBot==1.1.0a7
  - mathparse [required: >=0.1,<0.2, installed: 0.1.2]
  - python-dateutil [required: >=2.8,<2.9, installed: 2.8.2]
    - six [required: >=1.5, installed: 1.16.0]

High-level Goals

As the update manager effectively uses pipenv lock to update dependencies, the goal of this request is to let the manager run in scenarios where pipenv lock runs.

Proposal description

Do not block execution of pipenv resolution.

Alternatives

The alternative of leaving the restriction (no git, no local) in place is consistent with the requirements of the thoth-advise manager.

However, as pipenv itself does not impose the restrictions that thoth advise does, the current behaviour makes it inconsistent with pipenv.

Additional context

I did a quick search on when the check for git references was introduced, and AFAICT this was added in #820 to address https://github.com/thoth-station/support/issues/20

Acceptance Criteria

codificat commented 2 years ago

/sig user-experience

codificat commented 2 years ago

A suggestion from today's tech talk is: if we enable this, do it via a (new) configuration flag.

Gkrumbach07 commented 2 years ago

additional acceptance

shreekarSS commented 1 year ago

additional acceptance

  • [ ] should work on the OS climate demo repo

I tested on aicoe-osc-demo repo, able to create this PR https://github.com/shreekarSS/aicoe-osc-demo/pull/8 Issue raised https://github.com/os-climate/aicoe-osc-demo/issues/207

codificat commented 1 year ago

/triage accepted

codificat commented 1 year ago

/wg cnbi