-
Notifications
You must be signed in to change notification settings - Fork 93
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
proposal: meta package #4158
proposal: meta package #4158
Conversation
name = cylc-flow | ||
name = cylc |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Renames the PyPi distribution cylc-flow
-> cylc
.
f163ddf
to
a81fd07
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I am not one of the best to review this PR, but leaving my 0.02 cents here anyway 🙂
There are definitely pros and cons to having the option to mix and match the components of a Cylc installation IMO. While it may be convenient to have a single package that, when installed, will bring all the packages required for the Cylc system, I normally prefer being able to choose what I will install where.
That way I can customize an Ansible/K8s/Docker/Terraform deployment specifying which modules/packages are installed where. But that's just my preference, I think the best people to judge how Cylc should be installed are yourself, Davids, Hilary, and others supporting production environments.
JupyterHub is a good example for this too, I think. It allows you to install jupyterhub
Python package, the configurable-http-proxy
(or alternatives) using the NPM package manager. You can also install Postgres with the operating system package manager.
However, the equivalent of their metapackage is their Kubernetes Helm, which can be customized for things like which proxy to use (configurable-http-proxy or traefik).
Should we move in the future to allow users to install Cylc in containers, cloud services, or use any database provider, I think the pip
metapackage would be obsolete. The conda package could perhaps still be used.
This metapackage has cons as well. If jupyterhub
ever has a CVE reported for it, and it matches the version we are using in Cylc UI Server in version a.b.c, then we could create a CVE for Cylc UI Server x.y.1
, quickly release x.y.2
, and ask users to update to this version. Sysadmins wouldn't have to worry about hosts with cylc-flow
. With the metapackage, in this scenario, they would have to make sure that the cylc
package installed did not bring the cylc-uiserver
as an optional dependency (i.e. it has only the scheduler installed) (and I think the right thing would be to report the CVE against cylc-uiserver and against cylc metapackages, assuming we adopt the security workflow used in some other projects),
But said all that, I'm not maintaining Cylc in operations, and we will probably start thinking about containers, cloud, in a later version of Cylc 8 or Cylc 9, so we can try this different approach if others prefer.
In Conda Forge I think we would ignore the cylc-flow-feedstock recipe and artefact, and instead update conda-forge/cylc-feedstock
to install the python code from this repository and handle the multiple outputs in Conda Forge (which I have that WIP PR that can be used if helpful).
Bruno
# these are other cylc components within the cylc meta-package | ||
'uiserver': [ | ||
'cylc-uiserver==0.3.0' | ||
], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't it mean cylc
has a downstream dependency on cylc-uiserver
, and that cylc-uiserver
has an upstream dependency on cylc
now? Creating a circular dependency here?
Not sure whether it will always work (i.e. pip
, poetry
, pipenv
, tox
, setuptools
, importlib
; each of these could fail to handle a circular dependency I think? Looks like pip
has had some issues before - https://github.com/pypa/pip/issues?q=circular+dependency)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, I was thinking (not very deeply!) that we'd have to remove the cylc dependency from the UIS, but that doesn't make sense since it is a real code dependency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we are re-using the protobuf schema and some GraphQL code too (also some ID parsing maybe?).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The UI Server imports the Protobuf and GraphQL schema from cylc-flow directly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure whether circular imports are problematic, will have to do some reading.
'cylc-uiserver==0.3.0' | ||
], | ||
'rose': [ | ||
'cylc-rose==0.1.1' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ditto the above here, for circular dependency.
Thanks for that, I hadn't thought about circular imports.
I think whichever way around we do it you will still be able to mix and match components if you so choose: # 1 mix and match (separate meta package)
conda install cylc-flow cylc-uiserver
# 2 mix and match (integrated meta package)
conda install cylc cylc-uiserver And either way around you get into the same problem if you try installing from both the meta and plain package: $ conda install cylc cylc.uiserver
$ const install cylc-uiserver==<version>
version conflict between the cylc-uiserver version pinned by cylc.uiserver
and the one specified on the cli I.E. I don't think it actually makes a difference whether we use a separate metapackage or put optional deps into the main one. Because However, version pinning in the ui-server may mean that there are a very small number of possible combinations that would work since the ui-server will need to import the GraphQL and Protobuf schema from newer Cylc versions in order to be able to work with newer fields.
Not sure about the pip package, from the conda side Jupyterhub is a single feedstock with three outputs: # install jupyterhub & node & configurable-http-proxy
conda install jupyterhub
# install jupyterhub & nodebook
conda install jupyterhub-singleuser
# install jupyterhub only
conda install jupyterhub-base I'm not sure what the difference between one feedstock with multiple outputs and multiple feedstocks is? I think it's just less overhead having one feedstock and you avoid package vendoring? |
I find the idea of a blank meta-package which serves only to bring in other repos a little awkward since it's really just a strange way of sharing a recipe. However, I'm not sure how useful this actually is since
For example one site might want an environment that looks like this: name: cylc-8.0b0
channels:
- conda-forge
dependencies:
- python=3.7.10
- cylc-flow=8.0b0
- cylc-uiserver=0.3.0
- cylc-rose=0.1.1
- metomi-rose=2.0b1
- some-jupyterhub-auth-plugin
- some-jupyteerhub-spawner-plugin Sites might want to do things like:
E.G some other site might want: name: cylc-8.0b0
channels:
- conda-forge
- artifatory-internal
dependencies:
- bash=5
- coreutils
- python=3.9.2
- cylc-flow=8.0b0
- cylc-uiserver=0.3.0
- internal-jupyterhub-auth-plugin
- internal-cylc-plugin Another site might want to bring in dependencies from a commercially supported channel: name: cylc-8.0b0
channels:
- anaconda
dependencies:
- python=3.7.10 |
a81fd07
to
6a901f8
Compare
(meeting: @oliver-sanders to supersede this with a proposal for a simpler no-metapackage solution + document how to install the separate packages as required) |
@oliver-sanders - can we close this now? |
sibling: cylc/cylc-doc#234
Easier to code than to explain...
The Cylc "meta" package is intended to bundle together a set of Cylc components at specified versions to insure inter-compatibility, make installation easier and ensure consistency between site installations.
There are two approaches:
cylc.meta
module.Over discussions over the past year we have concluded:
We have been heading towards (1), however, considering the above I'm not convinced this is a good idea.
The only real advantage of (1) is that the user is more free to mix and match Cylc components (within the bounds of version pinning), which is more con than pro.
This PR implements (2), simply making the other Cylc components optional dependencies of cylc-flow. See how it would work in the docs PR - cylc/cylc-doc#234.
In Forge-land optional dependencies become package outputs (see Bruno's working example).
This is nice and simple, easy to maintain and keeps the PyPi/Forge versions nicely coupled. One problem, the "cylc-flow" package is now the "cylc" meta-package.
Proposal:
cylc
on PyPi and Forge rather thancylc-flow
cylc-flow
namespaces as best we can.cylc
🤦.TODO:
cylc-flow
references tocylc
Requirements check-list
CONTRIBUTING.md
and added my name as a Code Contributor.setup.py
, seerecipe/meta.yaml
).