pepper icon indicating copy to clipboard operation
pepper copied to clipboard

Modernize packaging

Open simmel opened this issue 6 years ago • 4 comments
trafficstars

I'm willing to do the work to modernize the packaging of pepper. Here's what we suggest:

  • Add a dependency on requests in setup.py so that pepper works out of the box after installation.
  • Add an "extra" for kerberos which also installs requests-gssapi when installing salt-pepper via pip install 'salt-pepper[kerberos]'.
  • Maybe even add an extra for test which depends on and installs pytest, tox, salt et.al?

The goal would be to make salt-pepper faster to bootstrap and ready to use out of the box after install.

simmel avatar Nov 15 '19 12:11 simmel

Simon, I’m not against your work but I think leaving requests as an extra would be ideal. I think the original intent of pepper was to be python-core dependencies only. Is a hard dep necessary?

On Fri, Nov 15, 2019 at 7:23 AM Simon Lundström [email protected] wrote:

I'm willing to do the work to modernize the packaging of pepper. Here's what we suggest:

The goal would be to make salt-pepper faster to bootstrap and ready to use out of the box after install.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/saltstack/pepper/issues/200?email_source=notifications&email_token=AAAK2VQ7T3GISGLCZWRODMLQT2IEVA5CNFSM4JN2DLO2YY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4HZTCQGA, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAK2VWKRZPQ2HGMORYWS73QT2IEVANCNFSM4JN2DLOQ .

mattp- avatar Nov 15 '19 22:11 mattp-

@mattp- Ah, it seems like requests is used in pepper/libpepper.py in the req_stream and req_get (and for the kerberos usage in req_requests) methods but everywhere else urllib is used. This complicates things. Any idea how to proceed? I'm willing to do the work.

simmel avatar Nov 26 '19 07:11 simmel

@simmel you could do your original approach, but make requests an extra similar to kerberos no?

side note, I took over maintainership of this package a little while ago. I guess I get the idea of keeping a lean dependency list, but not sure how important that is these days. Do you see an issue in adding requests as a dependency and junking urllib? its apache licensed so shouldn't be an issue for enterprise-y folk.

mattp- avatar Nov 26 '19 18:11 mattp-

On 26 Nov 2019, at 19:39, Matt Phillips [email protected] wrote: @simmel you could do your original approach, but make requests an extra similar to kerberos no?

Yes, but since this package is two fold (both a library and an CLI application) and only the library uses requests I find it hard to draw a line. How do we know if the person installing pepper is going to use it as CLI or library? 🤷‍♂️ Sure we could document that installing it as “pepper[library]” is needed for using it as a library. But then we’d have to remove the library when not installing it with that extra to not surprise users?

side note, I took over maintainership of this package a little while ago.

Ooh, nice! Congratulations!

I guess I get the idea of keeping a lean dependency list, but not sure how important that is these days.

Packaging was not then what it is today either. Much easier now with modern setup.py or poetry and virtualenv/pipx/pex/PyOxidizer.

Do you see an issue in adding requests as a dependency and junking urllib?

I (secretly hoped but) never thought you’d ask :) I think this is a great opportunity to clean up the code base as well!

simmel avatar Nov 27 '19 07:11 simmel