[v7r2] Fully support setuptools by chrisburr · Pull Request #4865 · DIRACGrid/DIRAC

chrisburr · 2020-12-10T08:04:12Z

BEGINRELEASENOTES

*Core
NEW: Fully support installation with setuptools

ENDRELEASENOTES

chrisburr

This is ready for feedback before I convert all the scripts. I've added some comments in a review to try and explain what is going on.

chrisburr · 2020-12-10T16:05:14Z

+package_dir=
+    =src
+packages = find:
+install_requires =


It would be useful if someone can take a look at the dependencies and see if I've missed anything or got the client/server/testing split wrong.

Version constraints should also be added before the first non-pre release but that can wait for a little while.

Just as a precision after discussion: only minimal version constraint is envisaged (e.g. M2Crypto>=0.36)

The stability will come from the fact that a given DIRACOS(2) version maps to a single set of packages. Adding restrictive versions here will just add a maintaince burdon and make it harder for people to actually install DIRAC alongside other things.

We have several of these lists now (environment.yml, environment-py3.yml, requirements.txt), would it be possible to reduce the number?

environment-py3.yml, requirements.txt, docs/requirements.txt and docs/requirements_py3.txt will disappear relatively soon. We will then be left with only three places:

setup.cfg: Runtime dependencies of DIRAC that are Python packages themselves, validated whenever DIRAC is installed

environment.yml: Any packages that need to be available to run the tests (excluding integration for now, though that's not actually a problem now as everything is available from conda-forge), i.e. not limited to just Python packages so this can included singularity/python itself/openssl/...

constructor.yml in DIRACOS2: Packages which will be included in the DIRACOS2 tarball

meta.yaml: Allows a conda package to be built for DIRAC, not a big deal as it can be automatically generated from the PyPI metadata, that is itself generated from the setup.cfg file.

While there will be some duplication between the above they each serve distinct roles any any effort to de-duplicate them further will be more work than it's worth.

constructor.yml will be a subset of environment.yml, right? And setup.cfg will also be a (different) subset of environment.yml.

Yes though package names in setup.cfg will be the names on PyPI which might differ slightly from the name of the conda-package. For example graphviz is the Python bindings on PyPI but the CLI application on conda-forge (python-graphviz is the conda-forge name for the Python bindings).

chrisburr · 2020-12-10T16:07:29Z

+from DIRAC import gLogger
+
+
+class DIRACScript(object):


This is the class that does the magic for replacing console_scripts functions with the highest priority extension. It does nothing when installed with Python 2 (under the assumption that it's a legacy install using PYTHONPATH).

chrisburr · 2020-12-10T16:09:30Z

+  All console-scripts entrypoints in DIRAC and downstream extensions should be
+  wrapped in this decorator to allow extensions to override any entry_point.
+  """
+  def __init__(self):


This constructor could be useful for allowing something like below. It's not strictly needed though so it can be deferred to another release if we want (or never). I'll wait for opinions before implementing it.

@DiracScript( switches=[ ("f:", "file=", "File to use as user key"), ("i", "version", "Print version"), ("n", "novoms", "Disable VOMS"), ("v", "checkvalid", "Return error if the proxy is invalid"), ("x", "nocs", "Disable CS"), ("e", "steps", "Show steps info"), ("j", "noclockcheck", "Disable checking if time is ok"), ("m", "uploadedinfo", "Show uploaded proxies info"), ], requriesProxy=False, ) def proxy_info(*args, **kwargs):

There's hardly any CLI not needed a proxy. But I admit that checking it before hand would give much more meaningfull messages. But that's maybe not the place to do it.
For the switches, maybe we could maybe look at something like click ? Anyway, I would differ it to later

I would imagine requriesProxy=True would be the default if we did this.

For the switches, maybe we could maybe look at something like click ? Anyway, I would differ it to later

I think the current parsing is too inconsistent to repliment using click so while it would be nice we need to be prepared to a single big backwards-incompatible break or, more likely, a transisition to a new CLI.

Anyway, I would differ it to later

In hindsight I agree that it's best left out of this PR to ensure the diff is reviewable.

chrisburr · 2020-12-10T16:14:37Z

@@ -1,72 +0,0 @@
-#!/usr/bin/env python


I've only modified three scripts for now as an example so people can give feedback without needing to duplicate the work. While the diff looks scary the actual change is:

Rename dirac-info.py to dirac_info.py so it's a valid Python module. This is safe as dirac-install already replaces _ with - when generating the wrapper scripts. (I was going to suggest implementing this but it was already there.)

Indent almost all lines so they can exist in a main() function

Add if __name__ == "__main__": main() to the end so the script can still be ran directly

chrisburr · 2020-12-10T16:20:14Z

+  return {
+      "priority": 0,
+      "setups": {
+          "DIRAC-Certification": {


This isn't used yet but it will be in my follow up PR to this one. I didn't mean to include it here but I see no harm in leaving it.

chaen

In general, I am extremely pleased by all that (although not very surprised since we did talked about it :D )

However, I would advocate for more explanations, not necessarily yet in the official wiki/ existing rst (this can come once everything is consolidated) but at least in a document that explains the different bits and pieces of the whole mechanism, with links to various documents (PEP or others). This is for the sake of keeping the global picture accessible to everybody

I would also keep somewhere (maybe this time in the official instructions) what needs to be done already (for example the double tagging), such that it is not overlooked

Thanks for the tedious work :-)

chaen · 2020-12-11T13:39:12Z

+    sqlalchemy
+    stomp.py
+    suds-jurko
+    tornado


I guess we need to specify the repo here?

I hope we can require the local version segment and use something like tornado =*+dirac.* but I'm not sure if that actually works properly. I need to do some tests here but in practice pip install DIRAC[server] is not going to work without some help from outside (i.e. DIRACOS2) as some of these packages don't even exist on PyPI and many don't provide wheels/robust enough build systems. Regardless I think including the git repo here is a bad idea.

chaen · 2020-12-11T14:43:50Z

+  All console-scripts entrypoints in DIRAC and downstream extensions should be
+  wrapped in this decorator to allow extensions to override any entry_point.
+  """
+  def __init__(self):


There's hardly any CLI not needed a proxy. But I admit that checking it before hand would give much more meaningfull messages. But that's maybe not the place to do it.
For the switches, maybe we could maybe look at something like click ? Anyway, I would differ it to later

fstagni · 2021-01-06T16:51:38Z

+package_dir=
+    =src
+packages = find:
+install_requires =


We have several of these lists now (environment.yml, environment-py3.yml, requirements.txt), would it be possible to reduce the number?

fstagni · 2021-01-06T16:54:17Z

+    parameterized
+    pytest
+
+[options.entry_points]


Can you add some comments here?
e.g. "what to do if you have a script with the same name in the extension"

I've added documentation in 905965f and a comment in the setup.cfg referencing it.

chrisburr · 2021-01-13T14:47:36Z

I think everything has been dealt with here and it's now ready for all of the scripts to be converted.

chrisburr · 2021-01-14T09:11:06Z

@chaen Can you look at 7de4904 to see if it's okay for the tornado scripts? If it isn't I'm going to have to add a hack in the DIRACScript() to make it work.

…tegration

…service.py

chrisburr · 2021-01-19T11:01:13Z

@chaen @fstagni Is there anything else to do before this can be approved/merged?

fstagni

I think I can approve, now it's about testing.

andresailer · 2021-01-19T11:47:30Z

I am wondering if there are going to be conflicts because of the changes in many scripts in #4853

chrisburr · 2021-01-19T12:06:07Z

I am wondering if there are going to be conflicts because of the changes in many scripts in #4853

It will be okay with the module docstring changes as those haven't moved. I suspect the code changes will basically need to be re-done from scratch in one of the PRs.

andresailer · 2021-01-19T15:42:00Z

+
+__RCSID__ = "$Id$"
+
+# Must be define BEFORE any dirac import


This comment belongs to the os.environ below

andresailer · 2021-01-19T15:47:51Z


+print("NOTE:", __file__, "is deprecated and will be removed in v7r3, for details see",
+      "https://github.com/DIRACGrid/DIRAC/wiki/DIRAC-v7r2#rename-of-scripts")
+


Why isn't the main() from dirac_agent.py called here?

This (i.e. copying the entire thing) was what we talked about during the BiLD but yeah I see no reason not to import instead 👍

[v7r2] Remaining review comments from #4865

chrisburr force-pushed the support-setuptools branch 5 times, most recently from f4b010e to 27ca8d8 Compare December 10, 2020 16:00

chrisburr commented Dec 10, 2020

View reviewed changes

chrisburr requested review from andresailer, chaen and fstagni and removed request for andresailer, chaen and fstagni December 11, 2020 10:33

chaen reviewed Dec 11, 2020

View reviewed changes

chrisburr force-pushed the support-setuptools branch from e08ac80 to 7baa497 Compare December 17, 2020 12:21

fstagni reviewed Jan 6, 2021

View reviewed changes

chrisburr force-pushed the support-setuptools branch 4 times, most recently from cf15fa3 to aacc335 Compare January 13, 2021 14:15

chrisburr force-pushed the support-setuptools branch from 99c232f to 20d9496 Compare January 13, 2021 19:58

chrisburr mentioned this pull request Jan 14, 2021

[v7r0] Modify __init__.py to avoid conflicts with rel-v7r2 #4894

Merged

chrisburr force-pushed the support-setuptools branch 2 times, most recently from 9edd08a to 9292153 Compare January 14, 2021 09:37

fstagni requested review from chaen and removed request for andresailer January 14, 2021 09:46

chrisburr added 14 commits January 19, 2021 06:54

Fix docs text formatting

35fa239

Rename all scripts to use _ instead of -

6505f58

PY3: Convery most scripts to use DiracScript decorator

d5ec9ff

Fix filename in py3CheckDirs.txt

704dfa0

PY3: Update ComponentInstaller to account for _ in script names

6567cc9

Use global variables in some required places

850a3d5

Fix typo

780368e

PY3: Convert dirac-platform and add shell scripts to setup.cfg

6ba6fb7

PY3: Convert tornado scripts to be setup.cfg compatible

88c6150

Fix formatting

fa95a5c

Restore dirac-agent.py dirac-executor.py and dirac-service.py from in…

f0392ea

…tegration

Add deprecation notice to dirac-agent.py dirac-executor.py and dirac-…

35931a0

…service.py

Improve console scripts documentation

29f6fa8

Add setuptools_scm comment in pyproject.toml

3b627c7

chrisburr force-pushed the support-setuptools branch from ea189d5 to 3b627c7 Compare January 19, 2021 05:54

fstagni approved these changes Jan 19, 2021

View reviewed changes

andresailer reviewed Jan 19, 2021

View reviewed changes

atsareg merged commit e2b3b61 into DIRACGrid:integration Jan 19, 2021

chrisburr added a commit to chrisburr/DIRAC that referenced this pull request Jan 20, 2021

Remaining review comments from DIRACGrid#4865

0e6ce2b

chrisburr added a commit to chrisburr/DIRAC that referenced this pull request Jan 20, 2021

Remaining review comments from DIRACGrid#4865

d3e05ce

chrisburr added a commit to chrisburr/DIRAC that referenced this pull request Jan 20, 2021

Remaining review comments from DIRACGrid#4865

17c1fd6

chrisburr added a commit to chrisburr/DIRAC that referenced this pull request Jan 20, 2021

Remaining review comments from DIRACGrid#4865

c685cda

chrisburr mentioned this pull request Jan 20, 2021

[v7r2] Remaining review comments from #4865 #4907

Merged

atsareg pushed a commit that referenced this pull request Jan 20, 2021

Merge pull request #4907 from chrisburr/simplify-runit-launchers

4f21e82

[v7r2] Remaining review comments from #4865

chrisburr deleted the support-setuptools branch January 21, 2021 06:54

chrisburr mentioned this pull request Feb 8, 2021

[v7r2] Revert scripts to before Python 3 incompatible merge #4942

Merged


		print("NOTE:", __file__, "is deprecated and will be removed in v7r3, for details see",
		"https://github.com/DIRACGrid/DIRAC/wiki/DIRAC-v7r2#rename-of-scripts")

Uh oh!

Conversation

chrisburr commented Dec 10, 2020

Uh oh!

chrisburr left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chaen left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

chrisburr commented Jan 13, 2021

Uh oh!

chrisburr commented Jan 14, 2021

Uh oh!

chrisburr commented Jan 19, 2021

Uh oh!

fstagni left a comment

Choose a reason for hiding this comment

Uh oh!

andresailer commented Jan 19, 2021

Uh oh!

chrisburr commented Jan 19, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!