This is an automated email from the ASF dual-hosted git repository. aviemzur pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/incubator-liminal.git
commit 027eb07b11795b12adada2b6e89de44b6235786a Author: michaelloewenstein <[email protected]> AuthorDate: Wed Jan 6 09:55:17 2021 +0200 [LIMINAL-5] change prints to log --- .../helloworld/hello_world.py | 8 +---- liminal/build/image_builder.py | 13 +++---- liminal/build/liminal_apps_builder.py | 21 +++++------ liminal/core/environment.py | 8 ++--- liminal/core/util/files_util.py | 5 +-- liminal/kubernetes/volume_util.py | 2 +- .../util/files_util.py => logging/__init__.py} | 13 ------- .../logging/logging_setup.py | 42 ++++++++++++---------- liminal/runners/airflow/dag/liminal_dags.py | 13 +++---- scripts/liminal | 20 +++++++---- setup.py | 4 +-- .../python/test_python_server_image_builder.py | 18 +++++----- .../build/python/test_python_image_builder.py | 5 +-- .../airflow/build/test_liminal_apps_builder.py | 1 - .../airflow/liminal/write_outputs/write_outputs.py | 5 +-- tests/runners/airflow/tasks/test_python.py | 3 +- 16 files changed, 91 insertions(+), 90 deletions(-) diff --git a/examples/liminal-getting-started/helloworld/hello_world.py b/examples/liminal-getting-started/helloworld/hello_world.py index 78fbd9c..a26752c 100644 --- a/examples/liminal-getting-started/helloworld/hello_world.py +++ b/examples/liminal-getting-started/helloworld/hello_world.py @@ -16,14 +16,8 @@ # specific language governing permissions and limitations # under the License. import json -import os print('Hello world!\n') -# with open('/mnt/gettingstartedvol/hello_world.json') as file: -# print(f'hello_world.json contents = {json.loads(file.readline())}') -# -# os.makedirs('/mnt/vol1/', exist_ok=True) - with open('/mnt/gettingstartedvol/hello_world_output.json', 'w') as file: - file.write(json.dumps({'hello': 1, 'world': 2})) \ No newline at end of file + file.write(json.dumps({'hello': 1, 'world': 2})) diff --git a/liminal/build/image_builder.py b/liminal/build/image_builder.py index a7ca13b..11ff9f0 100644 --- a/liminal/build/image_builder.py +++ b/liminal/build/image_builder.py @@ -16,6 +16,7 @@ # specific language governing permissions and limitations # under the License. +import logging import os import shutil import subprocess @@ -45,7 +46,7 @@ class ImageBuilder: """ Builds source code into an image. """ - print(f'[ ] Building image: {self.tag}') + logging.info(f'[ ] Building image: {self.tag}') temp_dir = self.__temp_dir() @@ -67,7 +68,7 @@ class ImageBuilder: if self._use_buildkit(): docker_build_command = f'DOCKER_BUILDKIT=1 {docker_build_command}' - print(docker_build_command) + logging.info(docker_build_command) docker_build_out = '' try: @@ -78,14 +79,14 @@ class ImageBuilder: docker_build_out = e.output raise e finally: - print('=' * 80) + logging.info('=' * 80) for line in str(docker_build_out)[2:-3].split('\\n'): - print(line) - print('=' * 80) + logging.info(line) + logging.info('=' * 80) self.__remove_dir(temp_dir) - print(f'[X] Building image: {self.tag} (Success).') + logging.info(f'[X] Building image: {self.tag} (Success).') return docker_build_out diff --git a/liminal/build/liminal_apps_builder.py b/liminal/build/liminal_apps_builder.py index dd98477..19b065f 100644 --- a/liminal/build/liminal_apps_builder.py +++ b/liminal/build/liminal_apps_builder.py @@ -16,6 +16,7 @@ # specific language governing permissions and limitations # under the License. +import logging import os import yaml @@ -31,7 +32,7 @@ def build_liminal_apps(path): config_files = files_util.find_config_files(path) for config_file in config_files: - print(f'Building artifacts for file: {config_file}') + logging.info(f'Building artifacts for file: {config_file}') base_path = os.path.dirname(config_file) @@ -51,7 +52,8 @@ def build_liminal_apps(path): else: raise ValueError(f'No such task type: {task_type}') else: - print(f'No source configured for task {task_name}, skipping build..') + logging.info( + f'No source configured for task {task_name}, skipping build..') if 'services' in liminal_config: for service in liminal_config['services']: @@ -72,7 +74,7 @@ def __build_image(base_path, builder_config, builder): tag=builder_config['image']) builder_instance.build() else: - print(f"No source provided for {builder_config['name']}, skipping.") + logging.info(f"No source provided for {builder_config['name']}, skipping.") def __get_task_build_class(task_type): @@ -83,13 +85,13 @@ def __get_service_build_class(service_type): return service_build_types.get(service_type, None) -print(f'Loading image builder implementations..') +logging.info(f'Loading image builder implementations..') # TODO: add configuration for user image builders package image_builders_package = 'liminal.build.image' # user_image_builders_package = 'TODO: user_image_builders_package' -task_build_classes = class_util.find_subclasses_in_packages( +TASK_BUILD_CLASSES = class_util.find_subclasses_in_packages( [image_builders_package], ImageBuilder) @@ -99,11 +101,10 @@ def get_types_dict(task_build_classes): return {x.split(".")[-2]: c for x, c in task_build_classes.items()} -task_build_types = get_types_dict(task_build_classes) +task_build_types = get_types_dict(TASK_BUILD_CLASSES) -print(f'Finished loading image builder implementations: {task_build_classes}') - -print(f'Loading service image builder implementations..') +logging.info(f'Finished loading image builder implementations: {TASK_BUILD_CLASSES}') +logging.info(f'Loading service image builder implementations..') # TODO: add configuration for user service image builders package service_builders_package = 'liminal.build.service' @@ -114,4 +115,4 @@ service_build_classes = class_util.find_subclasses_in_packages( ServiceImageBuilderMixin) service_build_types = get_types_dict(service_build_classes) -print(f'Finished loading service image builder implementations: {service_build_classes}') +logging.info(f'Finished loading service image builder implementations: {service_build_classes}') diff --git a/liminal/core/environment.py b/liminal/core/environment.py index 42e14a4..acea4e4 100644 --- a/liminal/core/environment.py +++ b/liminal/core/environment.py @@ -18,7 +18,7 @@ import os import subprocess -from pathlib import Path +import logging DEFAULT_DAGS_ZIP_NAME = 'liminal.zip' DEFAULT_LIMINAL_HOME = os.path.expanduser('~/liminal_home') @@ -29,8 +29,8 @@ LIMINAL_VERSION_PARAM_NAME = 'LIMINAL_VERSION' def get_liminal_home(): if not os.environ.get(LIMINAL_HOME_PARAM_NAME): - print("no environment parameter called LIMINAL_HOME detected") - print(f"registering {DEFAULT_LIMINAL_HOME} as the LIMINAL_HOME directory") + logging.info("no environment parameter called LIMINAL_HOME detected") + logging.info(f"registering {DEFAULT_LIMINAL_HOME} as the LIMINAL_HOME directory") os.environ[LIMINAL_HOME_PARAM_NAME] = DEFAULT_LIMINAL_HOME return os.environ.get(LIMINAL_HOME_PARAM_NAME, DEFAULT_LIMINAL_HOME) @@ -58,6 +58,6 @@ def get_liminal_version(): value = pip_res[pip_res.index(' @ ') + 3:] else: value = pip_res - print(f'LIMINAL_VERSION not set. Setting it to currently installed version: {value}') + logging.info(f'LIMINAL_VERSION not set. Setting it to currently installed version: {value}') os.environ[LIMINAL_VERSION_PARAM_NAME] = value return os.environ.get(LIMINAL_VERSION_PARAM_NAME, 'apache-liminal') diff --git a/liminal/core/util/files_util.py b/liminal/core/util/files_util.py index e611005..48ab9b7 100644 --- a/liminal/core/util/files_util.py +++ b/liminal/core/util/files_util.py @@ -17,14 +17,15 @@ # under the License. import os +import logging def find_config_files(path): files = [] - print(path) + logging.info(path) for r, d, f in os.walk(path): for file in f: if os.path.basename(file) in ['liminal.yml', 'liminal.yaml']: - print(os.path.join(r, file)) + logging.info(os.path.join(r, file)) files.append(os.path.join(r, file)) return files diff --git a/liminal/kubernetes/volume_util.py b/liminal/kubernetes/volume_util.py index 259faa8..eff1fd0 100644 --- a/liminal/kubernetes/volume_util.py +++ b/liminal/kubernetes/volume_util.py @@ -34,7 +34,7 @@ def create_local_volumes(liminal_config, base_dir): for volume_config in volumes_config: if 'local' in volume_config: - print(f'Creating local kubernetes volume if needed: {volume_config}') + logging.info(f'Creating local kubernetes volume if needed: {volume_config}') path = volume_config['local']['path'] if path.startswith(".."): path = os.path.join(base_dir, path) diff --git a/liminal/core/util/files_util.py b/liminal/logging/__init__.py similarity index 71% copy from liminal/core/util/files_util.py copy to liminal/logging/__init__.py index e611005..217e5db 100644 --- a/liminal/core/util/files_util.py +++ b/liminal/logging/__init__.py @@ -15,16 +15,3 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. - -import os - - -def find_config_files(path): - files = [] - print(path) - for r, d, f in os.walk(path): - for file in f: - if os.path.basename(file) in ['liminal.yml', 'liminal.yaml']: - print(os.path.join(r, file)) - files.append(os.path.join(r, file)) - return files diff --git a/tests/runners/airflow/liminal/write_outputs/write_outputs.py b/liminal/logging/logging_setup.py similarity index 50% copy from tests/runners/airflow/liminal/write_outputs/write_outputs.py copy to liminal/logging/logging_setup.py index 806ce0e..995fa02 100644 --- a/tests/runners/airflow/liminal/write_outputs/write_outputs.py +++ b/liminal/logging/logging_setup.py @@ -16,26 +16,32 @@ # specific language governing permissions and limitations # under the License. -import json -import os +import logging +from logging.handlers import RotatingFileHandler -split_id = int(os.environ['LIMINAL_SPLIT_ID']) -num_splits = int(os.environ['LIMINAL_NUM_SPLITS']) +from liminal.core import environment -inputs_dir = f'/mnt/vol1/inputs/{split_id}' -outputs_dir = '/mnt/vol1/outputs/' +LIMINAL = 'liminal' +LOGS_DIR = 'logs' +MAX_FILE_SIZE = 10485760 # 10 MB -if not os.path.exists(outputs_dir): - os.makedirs(outputs_dir) -print(f'Running write_outputs for split id {split_id} [NUM_SPLITS = {num_splits}]') +def logging_initialization(): + root_logger = logging.getLogger() -for filename in os.listdir(inputs_dir): - with open(os.path.join(inputs_dir, filename)) as infile, \ - open(os.path.join( - outputs_dir, - filename.replace('input', 'output').replace('.json', '.txt') - ), 'w') as outfile: - print(f'Writing output file: {outfile.name}') - data = json.loads(infile.read()) - outfile.write(data['mykey']) + log_formatter = logging.Formatter( + '[%(asctime)s] [%(filename)s:%(lineno)d] %(levelname)s - %(message)s', + '%m-%d %H:%M:%S' + ) + + file_handler = RotatingFileHandler( + f'{environment.get_liminal_home()}/{LOGS_DIR}/{LIMINAL}.log', + maxBytes=MAX_FILE_SIZE, + backupCount=3 + ) + root_logger.addHandler(file_handler) + root_logger.setLevel(logging.INFO) + + [h.setFormatter(log_formatter) for h in root_logger.handlers] + + logging.info('Logging initialization completed') diff --git a/liminal/runners/airflow/dag/liminal_dags.py b/liminal/runners/airflow/dag/liminal_dags.py index 3e5e1a3..03911b1 100644 --- a/liminal/runners/airflow/dag/liminal_dags.py +++ b/liminal/runners/airflow/dag/liminal_dags.py @@ -27,6 +27,7 @@ from liminal.core.util import files_util from liminal.runners.airflow.model.task import Task from liminal.runners.airflow.tasks.defaults.job_end import JobEndTask from liminal.runners.airflow.tasks.defaults.job_start import JobStartTask +import logging __DEPENDS_ON_PAST = 'depends_on_past' @@ -35,13 +36,13 @@ def register_dags(configs_path): """ Registers pipelines in liminal yml files found in given path (recursively) as airflow DAGs. """ - print(f'Registering DAG from path: {configs_path}') + logging.info(f'Registering DAG from path: {configs_path}') config_files = files_util.find_config_files(configs_path) dags = [] - print(f'found {len(config_files)} in path: {configs_path}') + logging.info(f'found {len(config_files)} in path: {configs_path}') for config_file in config_files: - print(f'Registering DAG for file: {config_file}') + logging.info(f'Registering DAG for file: {config_file}') with open(config_file) as stream: config = yaml.safe_load(stream) @@ -84,7 +85,7 @@ def register_dags(configs_path): job_end_task = JobEndTask(dag, config, pipeline, {}, parent, 'all_done') job_end_task.apply_task_to_dag() - print(f'registered DAG {dag.dag_id}: {dag.tasks}') + logging.info(f'registered DAG {dag.dag_id}: {dag.tasks}') globals()[pipeline_name] = dag dags.append(dag) @@ -92,7 +93,7 @@ def register_dags(configs_path): return dags -print(f'Loading task implementations..') +logging.info(f'Loading task implementations..') # TODO: add configuration for user tasks package impl_packages = 'liminal.runners.airflow.tasks' @@ -108,7 +109,7 @@ tasks_by_liminal_name = tasks_by_liminal_name( class_util.find_subclasses_in_packages([impl_packages], Task) ) -print(f'Finished loading task implementations: {tasks_by_liminal_name}') +logging.info(f'Finished loading task implementations: {tasks_by_liminal_name}') def get_task_class(task_type): diff --git a/scripts/liminal b/scripts/liminal index 3c7b132..ff4767e 100755 --- a/scripts/liminal +++ b/scripts/liminal @@ -18,6 +18,7 @@ # specific language governing permissions and limitations # under the License. +import logging import os import pathlib import shutil @@ -27,13 +28,16 @@ import sys import click import yaml -import scripts as s +import scripts from liminal.build import liminal_apps_builder from liminal.core import environment from liminal.core.util import files_util from liminal.kubernetes import volume_util +from liminal.logging.logging_setup import logging_initialization from liminal.runners.airflow import dag +logging_initialization() + try: import importlib.resources as pkg_resources except ImportError: @@ -64,6 +68,7 @@ def build(path): def deploy_liminal_core_internal(clean): + # noinspection PyTypeChecker with pkg_resources.path(dag, 'liminal_dags.py') as p: dags_path = p os.makedirs(environment.get_dags_dir(), exist_ok=True) @@ -72,13 +77,13 @@ def deploy_liminal_core_internal(clean): # initialize the env. variable which indicates to the docke compose which # liminal to install in airflow docker liminal_version = environment.get_liminal_version() - print(f'Deploying liminal version: {liminal_version}') + logging.info(f'Deploying liminal version: {liminal_version}') # if liminal is installed from local file - the developer needs to put it in the /scripts folder # in which case it will end up inside the container during build if liminal_version.find("file://") > -1: local_file_name = os.path.basename(liminal_version) full_path = os.path.join('/opt/airflow/dags', local_file_name) - print( + logging.info( f'Liminal was installed locally, changing the LIMINAL_VERSION parameter to {full_path}') os.environ[environment.LIMINAL_VERSION_PARAM_NAME] = full_path if clean: @@ -98,7 +103,7 @@ def docker_compose_command(command_name, args): '-p liminal --project-directory ' f'{project_dir} {command_name} {concated_args}' ] - print(run_command[0]) + logging.info(run_command[0]) if 'follow' in str(args): subprocess.call(run_command, env=os.environ, shell=True) return '', '' @@ -154,7 +159,7 @@ def logs(follow, tail): docker_compose_command('logs', ['--follow']) if tail > 0: stdout, stderr = docker_compose_command('logs', [f'--tail={tail}']) - print(stdout) + logging.info(stdout) @cli.command("start", @@ -162,7 +167,7 @@ def logs(follow, tail): "Make sure docker is running on your machine") def start(): liminal_version = environment.get_liminal_version() - print(f'starting liminal version: {liminal_version}') + logging.info(f'starting liminal version: {liminal_version}') if docker_is_running(): # initialize liminal home by default environment.get_liminal_home() @@ -170,7 +175,8 @@ def start(): def get_docker_compose_paths(): - with pkg_resources.path(s, 'docker-compose.yml') as p: + # noinspection PyTypeChecker + with pkg_resources.path(scripts, 'docker-compose.yml') as p: docker_compose_path = p project_dir = pathlib.Path(docker_compose_path).parent.parent.absolute() return docker_compose_path, project_dir diff --git a/setup.py b/setup.py index b06fd17..6bc9845 100644 --- a/setup.py +++ b/setup.py @@ -17,6 +17,7 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. +import logging import os import setuptools @@ -26,8 +27,7 @@ with open("README.md", "r") as fh: with open('requirements.txt') as f: requirements = f.read().splitlines() - print(requirements) - + logging.info(requirements) setuptools.setup( name="apache-liminal", diff --git a/tests/runners/airflow/build/http/python/test_python_server_image_builder.py b/tests/runners/airflow/build/http/python/test_python_server_image_builder.py index 18215d3..88f9663 100644 --- a/tests/runners/airflow/build/http/python/test_python_server_image_builder.py +++ b/tests/runners/airflow/build/http/python/test_python_server_image_builder.py @@ -17,6 +17,7 @@ # under the License. import json +import logging import os import threading import time @@ -26,8 +27,9 @@ from unittest import TestCase import docker -from liminal.build.service.python_server.python_server import PythonServerImageBuilder from liminal.build.python import PythonImageVersions +from liminal.build.service.python_server.python_server import PythonServerImageBuilder + class TestPythonServer(TestCase): @@ -81,7 +83,7 @@ class TestPythonServer(TestCase): time.sleep(5) - print('Sending request to server') + logging.info('Sending request to server') json_string = '{"key1": "val1", "key2": "val2"}' @@ -92,33 +94,33 @@ class TestPythonServer(TestCase): data=json_string.encode(encoding) ).read().decode(encoding)) - print(f'Response from server: {server_response}') + logging.info(f'Response from server: {server_response}') self.assertEqual(f'Input was: {json.loads(json_string)}', server_response) return build_out def __remove_containers(self): - print(f'Stopping containers with image: {self.image_name}') + logging.info(f'Stopping containers with image: {self.image_name}') all_containers = self.docker_client.containers matching_containers = all_containers.list(filters={'ancestor': self.image_name}) for container in matching_containers: container_id = container.id - print(f'Stopping container {container_id}') + logging.info(f'Stopping container {container_id}') self.docker_client.api.stop(container_id) - print(f'Removing container {container_id}') + logging.info(f'Removing container {container_id}') self.docker_client.api.remove_container(container_id) self.docker_client.containers.prune() def __run_container(self, image_name): try: - print(f'Running container for image: {image_name}') + logging.info(f'Running container for image: {image_name}') self.docker_client.containers.run(image_name, ports={'80/tcp': 9294}) except Exception as err: - print(err) + logging.exception(err) pass @staticmethod diff --git a/tests/runners/airflow/build/python/test_python_image_builder.py b/tests/runners/airflow/build/python/test_python_image_builder.py index 56667bd..7c0032a 100644 --- a/tests/runners/airflow/build/python/test_python_image_builder.py +++ b/tests/runners/airflow/build/python/test_python_image_builder.py @@ -15,6 +15,7 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. +import logging import os import shutil import tempfile @@ -41,7 +42,7 @@ class TestPythonImageBuilder(TestCase): self.__remove_dir(self.temp_airflow_dir) def test_build(self): - for python_version in [None , + for python_version in [None, PythonImageVersions().supported_versions[0]]: build_out = self.__test_build(python_version=python_version) self.assertTrue('RUN pip install -r requirements.txt' in build_out, @@ -103,7 +104,7 @@ class TestPythonImageBuilder(TestCase): docker_client.close() - print(container_log) + logging.info(container_log) self.assertEqual( "b'" diff --git a/tests/runners/airflow/build/test_liminal_apps_builder.py b/tests/runners/airflow/build/test_liminal_apps_builder.py index d8407e4..c011a5c 100644 --- a/tests/runners/airflow/build/test_liminal_apps_builder.py +++ b/tests/runners/airflow/build/test_liminal_apps_builder.py @@ -23,7 +23,6 @@ import docker from liminal.build import liminal_apps_builder - class TestLiminalAppsBuilder(TestCase): __image_names = [ 'my_python_task_img', diff --git a/tests/runners/airflow/liminal/write_outputs/write_outputs.py b/tests/runners/airflow/liminal/write_outputs/write_outputs.py index 806ce0e..4452187 100644 --- a/tests/runners/airflow/liminal/write_outputs/write_outputs.py +++ b/tests/runners/airflow/liminal/write_outputs/write_outputs.py @@ -17,6 +17,7 @@ # under the License. import json +import logging import os split_id = int(os.environ['LIMINAL_SPLIT_ID']) @@ -28,7 +29,7 @@ outputs_dir = '/mnt/vol1/outputs/' if not os.path.exists(outputs_dir): os.makedirs(outputs_dir) -print(f'Running write_outputs for split id {split_id} [NUM_SPLITS = {num_splits}]') +logging.info(f'Running write_outputs for split id {split_id} [NUM_SPLITS = {num_splits}]') for filename in os.listdir(inputs_dir): with open(os.path.join(inputs_dir, filename)) as infile, \ @@ -36,6 +37,6 @@ for filename in os.listdir(inputs_dir): outputs_dir, filename.replace('input', 'output').replace('.json', '.txt') ), 'w') as outfile: - print(f'Writing output file: {outfile.name}') + logging.info(f'Writing output file: {outfile.name}') data = json.loads(infile.read()) outfile.write(data['mykey']) diff --git a/tests/runners/airflow/tasks/test_python.py b/tests/runners/airflow/tasks/test_python.py index 418e2eb..1a91973 100644 --- a/tests/runners/airflow/tasks/test_python.py +++ b/tests/runners/airflow/tasks/test_python.py @@ -15,6 +15,7 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. +import logging import os import tempfile import unittest @@ -58,7 +59,7 @@ class TestPythonTask(TestCase): task1.apply_task_to_dag() for task in dag.tasks: - print(f'Executing task {task.task_id}') + logging.info(f'Executing task {task.task_id}') task.execute({}) inputs_dir = os.path.join(self.temp_dir, 'inputs')
