Create end point for events #9691

oyvindeide · 2025-01-09T09:26:25Z

Issue
Resolves #9001

Approach
Short description of the approach

(Screenshot of new behavior in GUI if applicable)

PR title captures the intent of the changes, and is fitting for release notes.
Added appropriate release note label
Commit history is consistent and clean, in line with the contribution guidelines.
Make sure unit tests pass locally after every commit (git rebase -i main --exec 'pytest tests/ert/unit_tests -n logical -m "not integration_test"')

When applicable

When there are user facing changes: Updated documentation
New behavior or changes to existing untested code: Ensured that unit tests are added (See Ground Rules).
Large PR: Prepare changes in small commits for more convenient review
Bug fix: Add regression test for the bug
Bug fix: Create Backport PR to latest release

codspeed-hq · 2025-01-09T09:49:43Z

CodSpeed Performance Report

Merging #9691 will improve performances by 10.62%

_{Comparing oyvindeide:add_websockets_endpoint (a0827e3) with main (ee6a12a)}

Summary

⚡ 1 improvements
✅ 24 untouched benchmarks

Benchmarks breakdown

	Benchmark	`BASE`	`HEAD`	Change
⚡	`test_load_from_context[gen_x: 20, sum_x: 20 reals: 10]`	7.1 ms	6.4 ms	+10.62%

src/ert/run_models/everest_run_model.py

DanSava

Some questions and comments

src/everest/bin/utils.py

DanSava · 2025-01-30T09:09:13Z

src/everest/detached/__init__.py

+                if message:
+                    event_dict = json.loads(message)
+                    if "snapshot" in event_dict:
+                        event_dict["snapshot"] = EnsembleSnapshot.from_nested_dict(
+                            event_dict["snapshot"]
+                        )
+                    try:
+                        event = EventWrapper(event=event_dict).event


If you rewrite this:

if message: event_dict = json.loads(message) if "snapshot" in event_dict: event_dict["snapshot"] = EnsembleSnapshot.from_nested_dict( event_dict["snapshot"] ) try: event = EventWrapper(event=event_dict).event

as

if message: try: event = status_event_from_json(message)

will make it a bit more clear and you no longer need EventWrapper

I get this then 🤔

pydantic_core._pydantic_core.ValidationError: 1 validation error for tagged-union[ForwardModelStepStart,ForwardModelStepRunning,ForwardModelStepSuccess,ForwardModelStepFailure,ForwardModelStepChecksum,RealizationPending,RealizationRunning,RealizationSuccess,RealizationFailed,RealizationTimeout,RealizationUnknown,RealizationWaiting,RealizationResubmit,EESnapshot,EESnapshotUpdate,EETerminated,EEUserCancel,EEUserDone,EnsembleStarted,EnsembleSucceeded,EnsembleFailed,EnsembleCancelled] Input tag 'FullSnapshotEvent' found using 'event_type' does not match any of the expected tags: 'forward_model_step.start', 'forward_model_step.running', 'forward_model_step.success', 'forward_model_step.failure', 'forward_model_step.checksum', 'realization.pending', 'realization.running', 'realization.success', 'realization.failure', 'realization.timeout', 'realization.unknown', 'realization.waiting', 'realization.resubmit', 'ee.snapshot', 'ee.snapshot_update', 'ee.terminated', 'ee.user_cancel', 'ee.user_done', 'ensemble.started', 'ensemble.succeeded', 'ensemble.failed', 'ensemble.cancelled' [type=union_tag_invalid, input_value={'iteration_label': 'Runn...e': 'FullSnapshotEvent'}, input_type=dict]

I checked out your branch and tested the replacement before I made the suggestion. When I did it again after the latest change I could still not reproduce the issue you encountered.

It is possible you replaced it with event_from_json(message) imported form _ert.events import event_from_json and not status_event_from_json from ert.run_models.event import status_event_from_json.

When I do the suggested above I get the same issue.

src/everest/detached/__init__.py

src/everest/detached/jobs/everserver.py

src/ert/run_models/everest_run_model.py

tests/everest/test_everserver.py

frode-aarstad · 2025-02-04T13:24:46Z

tests/everest/test_everserver.py

@@ -244,42 +192,22 @@ def mocked_server(url, verify, auth, timeout, proxies):
    # The server should fail and store a user-friendly message.
    assert status["status"] == ServerStatus.failed
    assert OPT_FAILURE_REALIZATIONS in status["message"]
-    assert "job1 Failed with: job 1 error 1" in status["message"]
-    assert "job1 Failed with: job 1 error 2" in status["message"]
-    assert "job2 Failed with: job 2 error 1" in status["message"]



Do we not have this information about the failure anymore?

We will if it exists in the snapshot, but the main difference here from before is that we dont read the error file from disk, as that relies more on the shared file system. If the information is in the snapshot, we will have it. If a job fails, the monitoring will give out whatever information is in the snapshot, so in this specific instance, if the snapshot property error had: job 2 error 1, that would be given. It would still be possible to read the error file and get the full error, but the reason for example ert does not do that, and the reason it is not in the snapshot is because it can be quite large. We could potentially tail the file, and it would be smaller. But that should be done on the server side then.

frode-aarstad · 2025-02-04T13:30:05Z

Seems like quite a few of the tests are testing less (asserting less) are we sure the changed tests tests everything we want ?

src/everest/detached/jobs/everserver.py

oyvindeide · 2025-02-05T08:27:53Z

Seems like quite a few of the tests are testing less (asserting less) are we sure the changed tests tests everything we want ?

Are they? If you mean: test_monitor_entry_no_show_all_jobs and test_monitor_entry_show_all_jobs they are replaced with faster unit tests checking just the monitor, the coverage should be the same, or slightly higher.

Running these tests will cause a lot of dangling threads because the server is started, but not stopped, leading to potential flakyness. This mocks the server completely, which has the drawback that there is less testing of the actual server, but that is also tested in other tests.

DanSava

Looks very good!

Not sure if this is a perception illusion because I didn't do any actual testing for this but it seems running the examples case in the cli seem to run faster.

It is unclear if it caused by the current changes but I noticed that test_simulator_cache is very slow. I will add an issue to investigate.

DanSava · 2025-02-07T08:50:08Z

src/everest/detached/__init__.py

+                if message:
+                    event_dict = json.loads(message)
+                    if "snapshot" in event_dict:
+                        event_dict["snapshot"] = EnsembleSnapshot.from_nested_dict(
+                            event_dict["snapshot"]
+                        )
+                    try:
+                        event = EventWrapper(event=event_dict).event


I checked out your branch and tested the replacement before I made the suggestion. When I did it again after the latest change I could still not reproduce the issue you encountered.

It is possible you replaced it with event_from_json(message) imported form _ert.events import event_from_json and not status_event_from_json from ert.run_models.event import status_event_from_json.

When I do the suggested above I get the same issue.

DanSava · 2025-02-07T09:23:36Z

I did more testing and test_simulator_cache is not just slow it tends to hang when running the test.

oyvindeide force-pushed the add_websockets_endpoint branch 4 times, most recently from 06dfbd1 to 79c008e Compare January 28, 2025 11:25

oyvindeide force-pushed the add_websockets_endpoint branch 2 times, most recently from c396805 to e734a28 Compare January 29, 2025 07:22

yngve-sk reviewed Jan 29, 2025

View reviewed changes

src/ert/run_models/everest_run_model.py Outdated Show resolved Hide resolved

oyvindeide force-pushed the add_websockets_endpoint branch from e734a28 to c373dd0 Compare January 29, 2025 14:43

DanSava reviewed Jan 30, 2025

View reviewed changes

oyvindeide force-pushed the add_websockets_endpoint branch 3 times, most recently from b9d4f73 to 66155f7 Compare January 31, 2025 08:04

DanSava reviewed Feb 3, 2025

View reviewed changes

src/ert/run_models/everest_run_model.py Outdated Show resolved Hide resolved

oyvindeide added 4 commits February 4, 2025 12:40

Fix bug where iteration was not set

e0290aa

Move function

3bbcbbb

Move variable

380310a

Fix typo

5fdd773

oyvindeide force-pushed the add_websockets_endpoint branch from 66155f7 to 6221630 Compare February 4, 2025 11:40

oyvindeide added 3 commits February 4, 2025 12:55

Create end point for events

20263a6

Add monitoring of failed jobs

c58f689

Decrease polling interval

c92e178

oyvindeide force-pushed the add_websockets_endpoint branch from 6221630 to c92e178 Compare February 4, 2025 11:55

oyvindeide self-assigned this Feb 4, 2025

frode-aarstad reviewed Feb 4, 2025

View reviewed changes

tests/everest/test_everserver.py Outdated Show resolved Hide resolved

frode-aarstad reviewed Feb 4, 2025

View reviewed changes

src/everest/detached/jobs/everserver.py Show resolved Hide resolved

oyvindeide added the release-notes:improvement Automatically categorise as improvement in release notes label Feb 5, 2025

oyvindeide added 2 commits February 5, 2025 14:21

Remove print statement

a0827e3

DanSava reviewed Feb 7, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create end point for events #9691

Create end point for events #9691

oyvindeide commented Jan 9, 2025 •

edited

Loading

codspeed-hq bot commented Jan 9, 2025 •

edited

Loading

DanSava left a comment

DanSava Jan 30, 2025

oyvindeide Feb 4, 2025

DanSava Feb 7, 2025

frode-aarstad Feb 4, 2025

oyvindeide Feb 5, 2025 •

edited

Loading

frode-aarstad commented Feb 4, 2025

oyvindeide commented Feb 5, 2025

DanSava left a comment

DanSava Feb 7, 2025

DanSava commented Feb 7, 2025

Create end point for events #9691

Are you sure you want to change the base?

Create end point for events #9691

Conversation

oyvindeide commented Jan 9, 2025 • edited Loading

When applicable

codspeed-hq bot commented Jan 9, 2025 • edited Loading

CodSpeed Performance Report

Merging #9691 will improve performances by 10.62%

Summary

Benchmarks breakdown

DanSava left a comment

Choose a reason for hiding this comment

DanSava Jan 30, 2025

Choose a reason for hiding this comment

oyvindeide Feb 4, 2025

Choose a reason for hiding this comment

DanSava Feb 7, 2025

Choose a reason for hiding this comment

frode-aarstad Feb 4, 2025

Choose a reason for hiding this comment

oyvindeide Feb 5, 2025 • edited Loading

Choose a reason for hiding this comment

frode-aarstad commented Feb 4, 2025

oyvindeide commented Feb 5, 2025

DanSava left a comment

Choose a reason for hiding this comment

DanSava Feb 7, 2025

Choose a reason for hiding this comment

DanSava commented Feb 7, 2025

oyvindeide commented Jan 9, 2025 •

edited

Loading

codspeed-hq bot commented Jan 9, 2025 •

edited

Loading

oyvindeide Feb 5, 2025 •

edited

Loading