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
Fix part of #9827: Migrate Google App Engine Taskqueue to Cloud Tasks #10699
Conversation
…hird-party-installation
…hird-party-installation
…hird-party-installation
… pip-tools to local machine
…hird-party-installation
…hird-party-installation
…hird-party-installation
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.
LGTM for the codeowner files
@@ -935,6 +935,7 @@ def test_untag_deleted_misconceptions_correctly_on_updating_skill(self): | |||
change_list, 'Delete misconceptions.') | |||
skill_fetchers.get_skill_by_id('skillid12345') | |||
self.process_and_flush_pending_tasks() |
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.
is this still needed?
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.
Ah good catch this was added as part of a merge. Removed.
Unassigning @DubeySandeep since they have already approved the PR. |
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.
LGTM!!
Unassigning @vojtechjelinek since they have already approved the PR. |
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.
LGTM as a codeowner!
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.
@U8NWXD PTAL! The PR is really large and new develop changes are repeatedly making the tests fail so I would like to merge this PR in asap. |
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.
lgtm
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.
Lgtm as codeowner
@sagangwee is out -- so, confirming no further reviewers are needed, and that this PR can be merged once all the CI checks pass. |
@seanlip Could you please merge this PR? Thanks! |
Overview
This PR migrates the taskqueue functionality used in Oppia to the new Cloud Tasks service. However, the GAE MapReduce and Pipeline libraries actually use the old appengine taskqueue api so the oppia app will be using both apis for a time. This is ok for the time being since both apis interact with the same environment as confirmed here:
https://cloud.google.com/tasks/docs/migrating
This PR also contains a task queue emulator for both unit test and development server task operations that will now be maintained by oppia developers since GAE no longer provides support for an emulator.
NOTE: This PR still has not passed linter checks or has complete 100% backend coverage(some tests are missing). However, this allows for a first pass review for any issues that need to be fixed immediately.
Essential Checklist
PR Pointers