Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Revert changes to job status #6969

Merged
merged 2 commits into from
May 15, 2024
Merged

Revert changes to job status #6969

merged 2 commits into from
May 15, 2024

Conversation

eradman
Copy link
Collaborator

@eradman eradman commented May 14, 2024

What type of PR is this?

  • Revert

Description

Revert in order to restore progressive job status

"Query in queue" should switch to "Executing query", but does not.

Commands used

git revert --no-commit bd17662005248c087a8d6aeadb866ae94d3d3465
git revert --no-commit 5ac5d86f5e4ae0adb0c296e53f95cb79e612237a
vim tests/handlers/test_query_results.py
git add tests/handlers/test_query_results.py

How is this tested?

  • Unit tests (pytest, jest)
  • E2E Tests (Cypress)
  • Manually

Related Tickets & Documents

#6964

@eradman
Revert changes to job status
149dd92 
Commands:

git revert --no-commit bd17662
git revert --no-commit 5ac5d86
vim tests/handlers/test_query_results.py
git add tests/handlers/test_query_results.py
@justinclift
Copy link
Member

@gaecoli Do you have time to look over this one, just to double check it's ok? 😄

gaecoli
gaecoli approved these changes May 15, 2024
View reviewed changes
Copy link
Member

@gaecoli gaecoli left a comment

Choose a reason for hiding this comment

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

It's ok.

@eradman eradman merged commit c874eb6 into getredash:master May 15, 2024
11 checks passed
@eradman eradman deleted the revert-job-status branch May 15, 2024 02:06
@eradman
Copy link
Collaborator Author

eradman commented May 15, 2024

Thanks for the review @gaecoli

The motive behind the commit this change reverts may have been good, but it wasn't tested well.

@gaecoli
Copy link
Member

gaecoli commented May 15, 2024

For major changes, we should introduce two or three people to review to avoid reverting code.

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

@justinclift justinclift justinclift left review comments

@gaecoli gaecoli gaecoli approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@eradman @justinclift @gaecoli