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

Separate core dependencies from deployment dependencies #89

Closed
jdemaeyer opened this issue Jul 8, 2020 · 5 comments
Closed

Separate core dependencies from deployment dependencies #89

jdemaeyer opened this issue Jul 8, 2020 · 5 comments

Comments

@jdemaeyer
Copy link
Owner

jdemaeyer commented Jul 8, 2020

With #82 we'll make brightsky available on PyPI. People who install this package will potentially only be interested in the parsing core, and there is no need to also install falcon, gunicorn, huey, psycopg2-binary, or sentry-sdk.

This requires making sure that all imports from these packages are lazy, and adding test environments that run specific tests without the optional dependencies installed.

@jdemaeyer
Copy link
Owner Author

After a closer look this does not seem worth it for now. We would

  • introduce a whole lot of lazy import clutter,
  • have to come up with the additional tox configuration to make sure the specific brightsky components really work without the extra dependencies, plus
  • have to work around pip-compile currently ignoring extras_require.

I may revisit this if there's interest from the community, but for now will let it rest. Here's a WIP patch modifying the setup.py and adding a pip-compile wrapper:

From 52127c14ed9ef3a645e3ebd44eaf486e3dc304fd Mon Sep 17 00:00:00 2001
From: Jakob de Maeyer <[email protected]>
Date: Wed, 26 Aug 2020 10:56:59 +0200
Subject: [PATCH] [WIP] Separate core, worker, and api dependencies

---
 requirements.txt                |  2 +-
 scripts/compile-requirements.py | 42 +++++++++++++++++++++++++++++++++
 setup.py                        | 23 ++++++++++++------
 3 files changed, 59 insertions(+), 8 deletions(-)
 create mode 100755 scripts/compile-requirements.py

diff --git a/requirements.txt b/requirements.txt
index 1c1d6a3..126a605 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -2,7 +2,7 @@
 # This file is autogenerated by pip-compile
 # To update, run:
 #
-#    pip-compile
+#    scripts/compile-requirements.py
 #
 astral==2.2               # via brightsky (setup.py)
 certifi==2020.6.20        # via requests, sentry-sdk
diff --git a/scripts/compile-requirements.py b/scripts/compile-requirements.py
new file mode 100755
index 0000000..11c5a21
--- /dev/null
+++ b/scripts/compile-requirements.py
@@ -0,0 +1,42 @@
+#!/usr/bin/env python
+
+# Temporary workaround until https://github.com/jazzband/pip-tools/issues/908
+# is resolved
+
+import os
+import sys
+import tempfile
+from distutils.core import run_setup
+
+from piptools.scripts import compile
+
+
+os.environ['CUSTOM_COMPILE_COMMAND'] = 'scripts/compile-requirements.py'
+
+
+dist = run_setup('setup.py', script_args=[])
+dependencies = set(dist.install_requires)
+for extra_dependencies in dist.extras_require.values():
+    dependencies.update(extra_dependencies)
+
+with tempfile.NamedTemporaryFile('w') as tmpfile:
+    tmpfile_name = tmpfile.name
+    tmpfile.write('\n'.join(dependencies))
+    tmpfile.flush()
+    try:
+        compile.cli([tmpfile_name, '-qo', 'requirements.txt'] + sys.argv[1:])
+    except SystemExit as e:
+        if e.code or '-h' in sys.argv or '--help' in sys.argv:
+            raise
+
+with open('requirements.txt') as f:
+    requirements_txt_in = f.readlines()
+
+requirements_txt_out = ''.join(
+    line.replace(f'-r {tmpfile_name}', 'brightsky (setup.py)')
+    for line in requirements_txt_in)
+print(requirements_txt_out, end='')
+
+if '-n' not in sys.argv and '--dry-run' not in sys.argv:
+    with open('requirements.txt', 'w') as f:
+        f.write(requirements_txt_out)
diff --git a/setup.py b/setup.py
index fb4d5b4..e1e42fe 100644
--- a/setup.py
+++ b/setup.py
@@ -28,17 +28,26 @@ setup(
     ],
     python_requires='>=3.8',
     install_requires=[
-        'astral',
         'click',
         'coloredlogs',
-        'falcon',
-        'falcon-cors',
-        'gunicorn',
-        'huey[redis]',
         'parsel',
-        'psycopg2-binary',
         'python-dateutil',
         'requests',
-        'sentry-sdk',
     ],
+    extras_require={
+        'worker': {
+            'huey[redis]',
+            'psycopg2-binary',
+        },
+        'api': {
+            'astral',
+            'falcon',
+            'falcon-cors',
+            'gunicorn',
+            'psycopg2-binary',
+        },
+        'sentry': {
+            'sentry-sdk',
+        }
+    },
 )
-- 
2.28.0

@jdemaeyer jdemaeyer removed their assignment Aug 26, 2020
@ntadej
Copy link

ntadej commented Nov 3, 2022

Hi @jdemaeyer, do you need any help with this? I want to use a more modern redis version in my code but it's conflicting with the one required by brightsky. I only need the parsing functionality.

@jdemaeyer
Copy link
Owner Author

@ntadej while the requirements are still not properly separated (but probably will be over the summer via #138), the brightsky package no longer forces a specific redis version (since v4 is now supported by huey). A new release will probably land on PyPI this week. :)

@jdemaeyer
Copy link
Owner Author

I believe the way forward here is to rip out all the parsing functionality (essentially all the classes in brightsky.parsers, including the unit conversions but excluding the downloaders and database exporters) into a separate Python package (very open to name suggestions!). That'll also give us a better place to support requests like #132. brightsky will then import and subclass the parsers.

@jdemaeyer
Copy link
Owner Author

Our parsing core is now available as a stand-alone package named dwdparse (GitHub, PyPI). 🎉

I refactored some of the parsing so dwdparse has no third-party dependencies at all, and – in what I would call one of my prouder moments 😅 – reduced the maximum memory usage while parsing MOSMIX_S from 4 GB to 30 MB...

It's currently released as version 0.9.x so that I have some wiggle room for changing the API while integrating it into Bright Sky (although I hope that won't be necessary). Once that's done I will roll it over to 1.x.x.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants