-
Notifications
You must be signed in to change notification settings - Fork 345
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
OPIK-992 automation fixes #1274
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a few minor comments. But this is good to go.
except Exception as _: | ||
pass |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You probably want to log at info level, so we can better debug it if something is unexpected.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these throwing an exception would not necessarily indicate something unexpected happening, just that it does not need to clean up the created database because it was deleted as part of the test. that said, i will be adding logging fixtures in my next ticket and documenting this too, so staying with this as is for now and adding loggers in the next PR.
Details
Various automation fixes for small changes to UI in prompt/dataset item tables, also more fixes for ensuring that tests run properly on cloud environments as well.
Pagination buttons issue turned out to be a problem with the newest version of Playwright, where some newly added checks for is_editable() now get called for is_enabled() too, rendering is_enabled() unusable on buttons. Reverted the playwright version and forcing it for now, will raise an issue on the Playwright repo.