remove screenshot in browser observation
Put all the removal of screenshot logic inside to_memory for browser obs.
Fixes #1553
Codecov Report
Attention: Patch coverage is 70.00000% with 9 lines in your changes are missing coverage. Please review.
:exclamation: No coverage uploaded for pull request base (
main@16400e4). Click here to learn what that means.
:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.
Additional details and impacted files
@@ Coverage Diff @@
## main #1588 +/- ##
=======================================
Coverage ? 62.59%
=======================================
Files ? 92
Lines ? 3735
Branches ? 0
=======================================
Hits ? 2338
Misses ? 1397
Partials ? 0
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Just updated!
On Sun, May 5, 2024 at 08:18 Robert Brennan @.***> wrote:
@.**** approved this pull request.
In opendevin/events/observation/browse.py https://github.com/OpenDevin/OpenDevin/pull/1588#discussion_r1590297727:
@@ -5,35 +5,65 @@ from .observation import Observation
+def _remove_fields(obj, fields: set[str]):
might make sense to put _remove_fields in the events folder so other actions/observations can use it
In opendevin/events/observation/browse.py https://github.com/OpenDevin/OpenDevin/pull/1588#discussion_r1590297789:
- elif isinstance(obj, list) or isinstance(obj, tuple):
for item in obj:_remove_fields(item, fields)- elif hasattr(obj, 'dataclass_fields'):
for field in fields:if field in obj.__dataclass_fields__:setattr(obj, field, None)for value in obj.__dict__.values():_remove_fields(value, fields)do we need this part? Seems like the first if covers everything
— Reply to this email directly, view it on GitHub https://github.com/OpenDevin/OpenDevin/pull/1588#pullrequestreview-2039675305, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABTNCYWLK43SUTGSJO7EOD3ZAYPSDAVCNFSM6AAAAABHHQMXZOVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDAMZZGY3TKMZQGU . You are receiving this because you authored the thread.Message ID: @.***>
Looks like a couple integration tests are unhappy for some reason...
Yeah my bad. Just fixed the error, plus fixing the frontend screenshot not showing up. @rbren
Thanks Frank!