search

equinor / webviz-ert

ERT webviz plugins
GNU General Public License v3.0
13 stars 25 forks source link

`before_first_request` is deprecated and is to be removed #397

Closed oysteoh closed 1 year ago

oysteoh commented 2 years ago

Running the tests in webviz causing this to happen

tests/views/test_state_saving.py: 2 warnings
  /prog/res/komodo/bleeding-py38-rhel7/root/lib/python3.8/site-packages/flask/scaffold.py:50: DeprecationWarning:

  'before_first_request' is deprecated and will be removed in Flask 2.3. Run setup code while creating the application instead.

and

tests/data/spe1_st/tests/test_webviz_ert.py::test_webviz_observation_analyzer
  /prog/res/komodo/bleeding-py38-rhel7/root/lib/python3.8/site-packages/flask/scaffold.py:50: DeprecationWarning:

  'before_first_request' is deprecated and will be removed in Flask 2.3. Run setup code while creating the application instead.

I'm not sure where this deprecation warning is raised from, it could be in the dash-framework and that there is nothing we can do but wait for them to patch. But - it should be looked into at least to make sure we are not the problem here.

DanSava commented 2 years ago

I looked a bit into it and the warning seems to be coming from the dash testing framework.

Even when using the most basic dash test from dash testing 

# 1. imports of your dash app
import dash
from dash import html
# 2. give each testcase a test case ID, and pass the fixture
# dash_duo as a function argument
def test_001_child_with_0(dash_duo):
    # 3. define your app inside the test function
    app = dash.Dash(__name__)
    app.layout = html.Div(id="nully-wrapper", children=0)
    # 4. host the app locally in a thread, all dash server configs could be
    # passed after the first app argument
    dash_duo.start_server(app)
    # 5. use wait_for_* if your target element is the result of a callback,
    # keep in mind even the initial rendering can trigger callbacks
    dash_duo.wait_for_text_to_equal("#nully-wrapper", "0", timeout=4)
    # 6. use this form if its present is expected at the action point
    assert dash_duo.find_element("#nully-wrapper").text == "0"
    # 7. to make the checkpoint more readable, you can describe the
    # acceptance criterion as an assert message after the comma.
    assert dash_duo.get_logs() == [], "browser console should contain no error"
    # 8. visual testing with percy snapshot
    dash_duo.percy_snapshot("test_001_child_with_0-layout")

We can reproduce the warning.

Not sure why when running all tests it seems the warning is coming from specific tests, it is not. it comes from all tests if they are ran individually

I think it is a good idea to keep this issue open to just track that this is patched in time or if not to just pin the dash version once Flask 2.3 hits us.

oysteoh commented 1 year ago

@valentin-krasontovitsch could you as our next release manager just verify / check if this is now solved in dash? I guess it is not very far into the future before we go to flask 2.3 and this will break if not fixed.

valentin-krasontovitsch commented 1 year ago

fixed in dash 2.7.0 according to changelog

valentin-krasontovitsch commented 1 year ago

should i specify the min version in webviz-ert's requirements? or should i just make sure that bleeding / my current beta komodo build have a higher version? where do we fix this stuff, locally on the project level or globally in komodo or both, and it depends?

valentin-krasontovitsch commented 1 year ago

( fyi bleeding has dash 2.7.0 but please still answer the question above if you will ^^ )

oysteoh commented 1 year ago

I think just bumping in bleeding and your release is sufficient! Usually we try to have as little pinning / restrictions on the project level as possible.

valentin-krasontovitsch commented 1 year ago

perfect, thanks! then i guess this issue can be closed?