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

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

Merged
merged 2 commits into from Oct 20, 2021

Conversation

matthewfeickert
Copy link
Collaborator

@matthewfeickert matthewfeickert commented Aug 18, 2021

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

try:
from pandaclient import PBookCore
except ImportError:
try:
from pandatools import PBookCore
except ImportError:
print("Failed to load PandaClient, please set up locally")
sys.exit(1)

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
Copy link
Collaborator Author

@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.

@matthewfeickert matthewfeickert changed the title build: Require panda-client 1.4.82 or later for API stability build: Require panda-client v1.4.82 or later for API stability Aug 18, 2021
@kratsg
Copy link
Contributor

kratsg commented Aug 19, 2021

confirmed

@dguest
Copy link
Owner

dguest commented Aug 19, 2021

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
Copy link
Collaborator Author

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

pandamonium/setup.cfg

Lines 42 to 43 in d392368

install_requires =
panda-client>=1.0

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
Copy link
Owner

dguest commented Aug 19, 2021

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
Copy link
Collaborator Author

matthewfeickert commented Aug 19, 2021

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.

@dguest dguest merged commit b8471cc into master Oct 20, 2021
@matthewfeickert matthewfeickert deleted the build/update-install-requires branch October 20, 2021 18:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants