Skip to content

Conversation

imenattatra
Copy link

@imenattatra imenattatra commented Jan 8, 2025

What type of PR is this?

the email that reports failed queries has its detail translated but its title has been missed , in this PR I add its translation and fix related tests.
I did not test this manually on staging since i will need to schedule emails that report failed queries and i don't want to invest a lot of time on it but it is a small change and it should work

Related to the translation project : https://github.com/metr-systems/backlog/issues/3476

How is this tested?

  • Unit tests (pytest, jest)

@imenattatra imenattatra requested review from a team and helenalebreton January 8, 2025 15:19
@imenattatra imenattatra self-assigned this Jan 8, 2025
@helenalebreton
Copy link

@imenattatra which report ist it exactly ? This report would generally never go to the client.

@imenattatra
Copy link
Author

@imenattatra which report ist it exactly ? This report would generally never go to the client.

it is the one we receive on gmail with title "Redash failed to execute X of your scheduled queries"... yeah it only goes to engineering

@helenalebreton
Copy link

@imenattatra which report ist it exactly ? This report would generally never go to the client.

it is the one we receive on gmail with title "Redash failed to execute X of your scheduled queries"... yeah it only goes to engineering

we might make our life more complicated if we translate that one right ? on the other hand, I am not sure how we want to do this since we are also looking at merge with Redash redash. What do you think ? metr team would prefer this email in english

self.assertEqual(1, f3["failure_count"])

def test_shows_latest_failure_time(self):
@mock.patch("redash.tasks.failure_report._", side_effect=lambda x: x)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not clear to me why we need this patch here. If the user doesn't have the translation configured, won't flask_babel already return the same string?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I gave it some time, I don't know why it fails finding the correct translation files... which does not happen when running the class test separately, I tried few things ... I could not manage to make it work yet but will keep it aside and return to it later

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it's a missing configuration when running the tests. When you come to it later you can look over a way to set the directory manually when running the tests by changing the conftest.py

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants