dguest / pandamonium

Command line scripts to parse panda web api
BSD 3-Clause "New" or "Revised" License
27 stars 17 forks source link

build: Require `panda-client` `v1.4.82` or later for API stability #62

Closed matthewfeickert closed 3 years ago

matthewfeickert commented 3 years ago

This PR requires a minimum required version of panda-client of 1.4.82 for API stability as a follow up PR to PR #58.

The backwards compatability that @kratsg added in PR #58

https://github.com/dguest/pandamonium/blob/d3923686fab6085c6e07f87e08af6c86bad8f69d/src/pandamonium/panda_kill_taskid.py#L12-L19

should be kept for sometime as at the moment panda-client is still older than v1.4.82 most places.

Example on ATLAS Connect:

$ ssh ATLASConnect 
Last login: Mon Jul 19 16:52:53 2021 from 23.249.32.244
Welcome to ATLAS Connect.

For registration or login problems:  support@connect.usatlas.org
ATLAS Connect user forum:  atlas-connect-l@lists.bnl.gov
For additional documentation and examples: http://connect.usatlas.org/
[feickert@login ~]$ setupATLAS 
lsetup               lsetup <tool1> [ <tool2> ...] (see lsetup -h):
 lsetup agis          ATLAS Grid Information System
 lsetup asetup        (or asetup) to setup an Athena release
 lsetup atlantis      Atlantis: event display
 lsetup eiclient      Event Index 
 lsetup emi           EMI: grid middleware user interface 
 lsetup ganga         Ganga: job definition and management client
 lsetup lcgenv        lcgenv: setup tools from cvmfs SFT repository
 lsetup panda         Panda: Production ANd Distributed Analysis
 lsetup pyami         pyAMI: ATLAS Metadata Interface python client
 lsetup root          ROOT data processing framework
 lsetup rucio         distributed data management system client
 lsetup views         Set up a full LCG release
 lsetup xcache        XRootD local proxy cache
 lsetup xrootd        XRootD data access
advancedTools        advanced tools menu
diagnostics          diagnostic tools menu
helpMe               more help
printMenu            show this menu
showVersions         show versions of installed software

[feickert@login ~]$ lsetup panda
************************************************************************
Requested:  panda ... 
 Setting up panda 1.4.81 ... 
>>>>>>>>>>>>>>>>>>>>>>>>> Information for user <<<<<<<<<<<<<<<<<<<<<<<<<
************************************************************************

Squash and merge commit message:

* Set lower bound on panda-client to v1.4.82 to ensure `pandaclient` API exists
   - c.f. https://github.com/PanDAWMS/panda-client/releases/tag/1.4.82
   - Amends PR #58
* Add Python 3.9 to Black target-versions
matthewfeickert commented 3 years ago

@kratsg if you can confirm that things are working for you I'll release v0.3.1 after this PR as I forgot to bump the minimum required panda-client for v0.3.0.

kratsg commented 3 years ago

confirmed

dguest commented 3 years ago

I agree with bumping the required version of the panda client: I have no idea what would work with version 1.0 (probably nothing).

Still, give that @kratsg specifically added a workaround so we can use 1.4.81 we should probably say we support it right? Bumping beyond that is telling users / admins that a newer version is required whereas what we want to say is that the older version is deprecated.

matthewfeickert commented 3 years ago

Still, give that @kratsg specifically added a workaround so we can use 1.4.81 we should probably say we support it right? Bumping beyond that is telling users / admins that a newer version is required whereas what we want to say is that the older version is deprecated.

@dguest Given the way that things work, anything in setup.cfg like

https://github.com/dguest/pandamonium/blob/d3923686fab6085c6e07f87e08af6c86bad8f69d/setup.cfg#L42-L43

only comes into play if you are pip installing pandamonium. If you are doing the "oldschool way" then setup.cfg never even gets used. Because of this, we would want to have backwards compatibility on pandas-client APIs until we're quite sure that all ATLAS cites show panda 1.4.82 or later following lsetup panda.

But if you are installing from PyPI then it is better to make it clear what are the lower bound of your dependencies and now we have a clear lower bound on using the supported APIs of panda-client v1.4.82. So this PR is only going to affect users like me that pip install pandamonium on their local machines and users like @kratsg that are pip installing it in Docker images.

If we actually want to have a deprecation warning then we should probably raise and then catch a DeprecationWarning exception.

dguest commented 3 years ago

Thanks for confirming that we only use setup.cfg for pip installs, but my point was more that if someone has a reason to use panda-client 1.4.81 that's fine (even with the pip install). Of course they should probably take the newer one, but I think taking the newest tag is the default behavior for pip anyway, right? So unless I'm mistaken, setting the requirement to 1.4.82 is a more forceful way to update people but not the only way: they will pull the more recent version by default.

That being said, I don't really see a problem with either approach, I'll merge this if that's what you think is best.

matthewfeickert commented 3 years ago

Thanks for confirming that we only use setup.cfg for pip installs, but my point was more that if someone has a reason to use panda-client 1.4.81 that's fine (even with the pip install). Of course they should probably take the newer one, but I think taking the newest tag is the default behavior for pip anyway, right?

Correct, pip will figure out the dependency lower (and upper) bounds and then install the latest release that passes them.

So unless I'm mistaken, setting the requirement to 1.4.82 is a more forceful way to update people but not the only way: they will pull the more recent version by default.

I view it as giving the users an assurance: This library is known to safely work with this lower bound of this dependency. This can change in the future as needed, but when you regularly test and update your lower bounds then you know that the releases that people are getting on PyPI will work when in the worst case scenario they or pip is required to fall back to this lowest supported release.

In general trying to support multiple incompatible versions of APIs in your dependencies is a bad place to be as a library and can snowball, especially if your dependencies aren't going to try to follow SemVer in practice or give deprecation warnings in advance. Trying to have releases like v0.3.0 that would say anything above panda-client v1.0.0 as valid (so including v1.4.81) make it possible for pip to realize that it should install v0.3.0 if there is an explicit requirement on panda-client<1.4.82 and allow for v0.3.1 and later to still work fine. But without updating lower bounds pip can't do the dependency solving for you.