Creates voiceover migration job #20165

Nik-09 · 2024-04-15T18:54:16Z

Overview

The PR creates beam job for voiceover migration i.e., copying the exiting voiceover data from curated exploration (ExplorationModel) to new storage system for voiceovers (EntityVoiceoversModel).

Note: Due to manual dependency on voiceover admins i.e., they should assign accents to voice artists before EntityVoiceoversModel creation. Hence we can’t automatically create EntityVoiceoversModel when a new voiceover is added, after the voiceover migration job runs (this PR).
One solution we are accepting is to make the UI ready for creators and learners (upcoming PR) so that after a successful run of this job, the release coordinator enables the feature flag so that all later voiceovers will be directly added to EntityVoiceoversModel instead of ExplorationModel.
This job is safe to merge because successively running the job updates the old data that are stored in the EntityVoiceoversmodel.
Some of the post-migration scenarios are discussed in this document making this PR safe to merge.

Essential Checklist

Please follow the instructions for making a code change.

I have linked the issue that this PR fixes in the "Development" section of the sidebar.
I have checked the "Files Changed" tab and confirmed that the changes are what I want to make.
I have written tests for my code.
The PR title starts with "Fix #bugnum: " or "Fix part of #bugnum: ...", followed by a short, clear summary of the changes.
I have assigned the correct reviewers to this PR (or will leave a comment with the phrase "@{{reviewer_username}} PTAL" if I can't assign them directly).

Testing doc (for PRs with Beam jobs that modify production server data)

A testing doc has been written: here
(To be confirmed by the server admin) All jobs in this PR have been verified on a live server, and the PR is safe to merge.

Proof that changes are correct

voiceover_migration.mp4

Proof of changes on desktop with slow/throttled network

Proof of changes on mobile phone

Proof of changes in Arabic language

PR Pointers

Never force push! If you do, your PR will be closed.
To reply to reviewers, follow these instructions: https://github.com/oppia/oppia/wiki/Make-a-pull-request#step-5-address-review-comments-until-all-reviewers-approve
Some e2e tests are flaky, and can fail for reasons unrelated to your PR. We are working on fixing this, but in the meantime, if you need to restart the tests, please check the "If your build fails" wiki page.
See the Code Owner's wiki page for what code owners will expect.

…r_voice_artists

seanlip

Just one comment, everything else looks good. Thanks @Nik-09!

seanlip · 2024-05-10T14:40:36Z

core/domain/voiceover_services.py

 ) -> voiceover_models.EntityVoiceoversModel:
-    """Creates an entity voiceovers model instance.
+    """Creates an entity voiceovers model instance without putting it into


Creates --> Creates and returns

Put a comma after "instance"

seanlip · 2024-05-10T14:43:51Z

core/domain/voiceover_services.py

+    """
+    entity_voiceovers_dict = entity_voiceovers.to_dict()
+
+    entity_id = entity_voiceovers_dict['entity_id']


Per this comment: #20165 (comment)

I see, this is an interesting use case for to_dict().

I think the part I'm confused about is why we can't write entity_id = entity_voiceovers.id or similar, and same for a bunch of the others below. I suggest you use the domain object where possible, and use the to_dict() for voiceovers_mapping only.

Signed-off-by: Nik-09 <nikhil.agarwal.2019@gmail.com>

Nik-09

Hi @seanlip I have addressed the comments, PTAL once you are free.
Thank you.

seanlip

Thanks @Nik-09! LGTM.

seanlip · 2024-05-11T15:24:39Z

@chris7716 @JayVivarekar Could one of you please put a review hold on this for the server admin test of the job? Thanks.

seanlip · 2024-05-11T15:25:52Z

@Nik-09 Just to check: why is the second checkbox in "Testing doc (for PRs with Beam jobs that modify production server data)" ticked? Has the job been run on the server? The PR only just got approved so I'm a bit surprised by that.

oppiabot · 2024-05-11T15:27:00Z

Hi @Nik-09, this PR is ready to be merged. Please address any remaining comments prior to merging, and feel free to merge this PR once the CI checks pass and you're happy with it. Thanks!

Nik-09 · 2024-05-12T04:49:51Z

@Nik-09 Just to check: why is the second checkbox in "Testing doc (for PRs with Beam jobs that modify production server data)" ticked? Has the job been run on the server? The PR only just got approved so I'm a bit surprised by that.

Unticked.

Nik-09 · 2024-05-13T12:24:29Z

I have completed the job run in the backup server. @seanlip can you please verify the data in the datastore and merge this PR. Thank you

seanlip · 2024-05-13T12:25:30Z

Datastore looks good; merging. Thanks!

Nik-09 added 30 commits March 10, 2024 13:38

Adds flag to prevent voiceover contribution

7c236f6

Adds feature flag to add voiceover with accent.

4f3ee43

Fixes lint issues

314dee5

Fixes lint issues

983b6aa

Fixes merge conflict

1559ee2

Fixes lint, mypy, prettier, frontend, backend tests

ae3242e

updates variable name

c2521bd

Updates UI

0d21042

updates variable name

326e6b3

Fixes backend test

9a87a36

Removes platform param variable

9ff7940

Removes redundant code.

aacd470

Updates flag name.

7be470d

Merge branch 'develop' of https://github.com/oppia/oppia into flag_fo…

8e9d7c8

…r_voice_artists

Fixes frontend tests

50f301b

Merge branch 'develop' of https://github.com/oppia/oppia into flag_fo…

3008083

…r_voice_artists

Adds helper method to enable voiceover contribution feature flag.

53bd79b

Fixes e2e test

dad1c6b

Merge branch 'develop' of https://github.com/oppia/oppia into flag_fo…

2c11b65

…r_voice_artists

Merge branch 'develop' of https://github.com/oppia/oppia into flag_fo…

bf61246

…r_voice_artists

Merge branch 'develop' of https://github.com/oppia/oppia into flag_fo…

c7d8af1

…r_voice_artists

Fixes e2e test

f7818be

Enabled video recording

4992c34

Merge branch 'develop' of https://github.com/oppia/oppia into flag_fo…

853765f

…r_voice_artists

Fixes E2E test

0c1c326

Merge branch 'develop' of https://github.com/oppia/oppia into flag_fo…

3eed3ab

…r_voice_artists

Updates feature flag name from snake case to camel case

aad85a5

Merge branch 'develop' of https://github.com/oppia/oppia into flag_fo…

6914e8c

…r_voice_artists

Updates feature flag for voiceovers

23e20df

Enabled video recording for debugging e2e.

1e16a52

Nik-09 assigned seanlip and unassigned Nik-09 May 9, 2024

seanlip reviewed May 10, 2024

View reviewed changes

seanlip assigned Nik-09 and unassigned seanlip May 10, 2024

updates code docstring

fdb8413

Signed-off-by: Nik-09 <nikhil.agarwal.2019@gmail.com>

Nik-09 assigned seanlip and unassigned Nik-09 May 11, 2024

Nik-09 commented May 11, 2024

View reviewed changes

seanlip approved these changes May 11, 2024

View reviewed changes

seanlip assigned chris7716 and unassigned seanlip May 11, 2024

oppiabot bot added the PR: LGTM label May 11, 2024

oppiabot bot assigned Nik-09 May 11, 2024

Nik-09 assigned seanlip and unassigned chris7716 and Nik-09 May 13, 2024

seanlip added this pull request to the merge queue May 13, 2024

github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 13, 2024

Nik-09 added this pull request to the merge queue May 13, 2024

Merged via the queue into oppia:develop with commit 2f1ae2e May 13, 2024
81 checks passed

Nik-09 deleted the voiceover_migration_job branch May 13, 2024 15:10

Nik-09 unassigned seanlip May 13, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Creates voiceover migration job #20165

Creates voiceover migration job #20165

Nik-09 commented Apr 15, 2024 •

edited by seanlip

seanlip left a comment

seanlip May 10, 2024

Nik-09 May 11, 2024

seanlip May 10, 2024

Nik-09 May 11, 2024

Nik-09 left a comment

seanlip left a comment

seanlip commented May 11, 2024

seanlip commented May 11, 2024 •

edited

oppiabot bot commented May 11, 2024

Nik-09 commented May 12, 2024

Nik-09 commented May 13, 2024

seanlip commented May 13, 2024

Creates voiceover migration job #20165

Creates voiceover migration job #20165

Conversation

Nik-09 commented Apr 15, 2024 • edited by seanlip

Overview

Essential Checklist

Testing doc (for PRs with Beam jobs that modify production server data)

Proof that changes are correct

Proof of changes on desktop with slow/throttled network

Proof of changes on mobile phone

Proof of changes in Arabic language

PR Pointers

seanlip left a comment

Choose a reason for hiding this comment

seanlip May 10, 2024

Choose a reason for hiding this comment

Nik-09 May 11, 2024

Choose a reason for hiding this comment

seanlip May 10, 2024

Choose a reason for hiding this comment

Nik-09 May 11, 2024

Choose a reason for hiding this comment

Nik-09 left a comment

Choose a reason for hiding this comment

seanlip left a comment

Choose a reason for hiding this comment

seanlip commented May 11, 2024

seanlip commented May 11, 2024 • edited

oppiabot bot commented May 11, 2024

Nik-09 commented May 12, 2024

Nik-09 commented May 13, 2024

seanlip commented May 13, 2024

Nik-09 commented Apr 15, 2024 •

edited by seanlip

seanlip commented May 11, 2024 •

edited