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

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

Closed
1 task
Tracked by #401
codificat opened this issue Aug 31, 2022 · 6 comments · Fixed by #1140
Closed
1 task
Tracked by #401

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

codificat opened this issue Aug 31, 2022 · 6 comments · Fixed by #1140
Assignees
Labels
kind/feature Categorizes issue or PR as related to a new feature. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. sig/user-experience Issues or PRs related to the User Experience of our Services, Tools, and Libraries. triage/accepted Indicates an issue or PR is ready to be actively worked on.

Comments

@codificat
Copy link
Member

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 AICoE/sefkhet-abwy#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 thoth-station/support#20

Acceptance Criteria

  • kebechet update manager works on any Pipfile or requirements.txt where pipenv lock does, without additional restrictions
@codificat codificat added the kind/feature Categorizes issue or PR as related to a new feature. label Aug 31, 2022
@codificat
Copy link
Member Author

/sig user-experience

@sesheta sesheta added the sig/user-experience Issues or PRs related to the User Experience of our Services, Tools, and Libraries. label Aug 31, 2022
@codificat codificat added this to New in SIG-User-Experience via automation Aug 31, 2022
@codificat
Copy link
Member Author

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

@Gkrumbach07 Gkrumbach07 added the priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. label Sep 6, 2022
@Gkrumbach07
Copy link
Member

additional acceptance

  • should work on the OS climate demo repo

@Gkrumbach07 Gkrumbach07 changed the title Allow local and git package refereces in update manager [3pt] Allow local and git package refereces in update manager Sep 6, 2022
@Gkrumbach07 Gkrumbach07 changed the title [3pt] Allow local and git package refereces in update manager [3 OR 5pt] Allow local and git package refereces in update manager Sep 6, 2022
@Gkrumbach07 Gkrumbach07 changed the title [3 OR 5pt] Allow local and git package refereces in update manager [3pt] Allow local and git package refereces in update manager Sep 6, 2022
@Gkrumbach07 Gkrumbach07 moved this from New to Next in SIG-User-Experience Sep 6, 2022
@shreekarSS
Copy link
Member

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 os-climate/aicoe-osc-demo#207

@codificat
Copy link
Member Author

/triage accepted

@sesheta sesheta added the triage/accepted Indicates an issue or PR is ready to be actively worked on. label Sep 19, 2022
@codificat
Copy link
Member Author

/wg cnbi

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. sig/user-experience Issues or PRs related to the User Experience of our Services, Tools, and Libraries. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
Status: Done
Status: Done
Development

Successfully merging a pull request may close this issue.

4 participants