Skip to content

Commit

Permalink
fix: warn user when calling load config from flow instance (#4787)
Browse files Browse the repository at this point in the history
  • Loading branch information
samsja committed May 19, 2022
1 parent c88d2cf commit 3fb3b41
Show file tree
Hide file tree
Showing 5 changed files with 40 additions and 10 deletions.
14 changes: 14 additions & 0 deletions jina/orchestrate/flow/base.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
import argparse
import base64
import copy
import inspect
import json
import multiprocessing
import os
import sys
import threading
import time
import uuid
import warnings
from collections import OrderedDict
from contextlib import ExitStack
from typing import (
Expand Down Expand Up @@ -2037,3 +2039,15 @@ def update_network_interface(self, **kwargs):
:param kwargs: new network settings
"""
self._common_kwargs.update(kwargs)

def __getattribute__(self, item):
obj = super().__getattribute__(item)

if (
item == 'load_config' and inspect.ismethod(obj) and obj.__self__ is Flow
): # check if obj load config call from an instance and not the Class
warnings.warn(
"Calling load_config from a Flow instance will override all of the instance's initial parameters. We recommend to use `Flow.load_config(...)` instead"
)

return obj
1 change: 1 addition & 0 deletions tests/integration/issues/github_4773/config.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
jtype: Flow
16 changes: 16 additions & 0 deletions tests/integration/issues/github_4773/test_load_config.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import warnings

import pytest

from jina import Flow


def test_load_config_instance():
with pytest.warns(UserWarning): # assert that a warning has been emited
f = Flow().load_config('config.yaml')


def test_load_config_class(): # assert that no warnings were emited
with warnings.catch_warnings():
warnings.simplefilter("error")
f = Flow.load_config('config.yaml')
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@

import pytest

from jina import Executor, Client, requests
from jina import Flow, Document
from jina import Client, Document, Executor, Flow, requests

cur_dir = os.path.dirname(os.path.abspath(__file__))
exposed_port = 12345
Expand All @@ -13,7 +12,7 @@
def flow(request):
flow_src = request.param
if flow_src == 'flow-yml':
return Flow().load_config(os.path.join(cur_dir, 'flow.yml'))
return Flow.load_config(os.path.join(cur_dir, 'flow.yml'))
elif flow_src == 'uses-yml':
return Flow(port=exposed_port).add(
uses=os.path.join(cur_dir, 'default_config.yml'),
Expand Down
14 changes: 7 additions & 7 deletions tests/unit/parsers/test_warnings.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import pytest

from jina import Flow, Executor
from jina import Executor, Flow


class MyExecutor(Executor):
Expand All @@ -16,7 +16,7 @@ def test_flow_with_warnings():
foo: bar
'''
with pytest.warns(UserWarning, match='ignored unknown') as record:
Flow().load_config(yaml)
Flow.load_config(yaml)
assert len(record) == 1
assert record[0].message.args[0].startswith('ignored unknown')

Expand All @@ -28,7 +28,7 @@ def test_executor_warnings():
- foo: bar
'''
with pytest.warns(UserWarning, match='ignored unknown') as record:
Flow().load_config(yaml)
Flow.load_config(yaml)
assert len(record) == 1
assert record[0].message.args[0].startswith('ignored unknown')

Expand All @@ -41,7 +41,7 @@ def test_executor_with_warnings():
foo: bar
'''
with pytest.warns(UserWarning, match='ignored unknown') as record:
Flow().load_config(yaml)
Flow.load_config(yaml)
assert len(record) == 1
assert record[0].message.args[0].startswith('ignored unknown')

Expand All @@ -54,7 +54,7 @@ def test_executor_uses_with_works():
foo: bar
'''
with pytest.warns(None, match='ignored unknown') as record:
Flow().load_config(yaml)
Flow.load_config(yaml)
assert len(record) == 0


Expand All @@ -65,7 +65,7 @@ def test_executor_override_with_warnings():
- override_with: 1
'''
with pytest.warns(None, match='ignored unknown') as record:
Flow().load_config(yaml)
Flow.load_config(yaml)
assert len(record) == 1


Expand All @@ -79,7 +79,7 @@ def test_executor_metas_works():
name: MyExecutor
'''
with pytest.warns(None, match='ignored unknown') as record:
with Flow().load_config(yaml):
with Flow.load_config(yaml):
pass
assert len(record) == 0

Expand Down

0 comments on commit 3fb3b41

Please sign in to comment.