Introduce asset download script [Blocked: #4937] #4885
Draft
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is utilizing a new Oppia web API being worked on that will allow for an easier collection of Oppia assets as part of a broader initiative to fully automate Oppia Android's release pipeline. There are a bunch of TODOs, documentation, testing, and functionality work needed yet before this solution is ready to check in (including introducing new proto to legacy proto conversion & outputting for the app to actually be able to consume the lessons being imported from Oppia web). This also sets the groundwork for downloads infrastructure both for the client & Oppia web's new proto APIs that will be introduced as part of itnroducing lessond download support in the app.
|
Hi @BenHenning, I'm going to mark this PR as stale because it hasn't had any updates for 7 days. If no further activity occurs within 7 days, it will be automatically closed so that others can take up the issue. |
oppiabot
bot
added
the
stale
Specifically, adds support for: - Embedding the API secret in the request header. - Supporting a batch-API that allows multiple structures to be requested simultaneouusly. Note that while the script now supports batch retrieval, it doesn't actually make use of it yet (since there's not an obvious way to test this without production data, so this will be added later).
oppiabot
bot
removed
the
stale
|
Hi @BenHenning, I'm going to mark this PR as stale because it hasn't had any updates for 7 days. If no further activity occurs within 7 days, it will be automatically closed so that others can take up the issue. |
oppiabot
bot
added
the
stale
Specifically: - Temporarily disables version fallback (to detect issues with the script). - Fixed multiple incompatibility issues, including adding a couple of temporary exemptions for the upcoming release. This also included fixing the invalid HTML tag regex pattern. - Disabled classroom support since the production math classroom isn't yet available via the new classroom API. - Added strong progress tracking for all expensive operations for a much easier time when using the script. - Added on-disk server result caching for much faster subsequent runs (and to put much less stress on the remote server). This included adding support for converting downloaded structures back to JSON. - Enabled image downloading. - Added outputting proto v2 pb and textproto files (legacy proto conversion still needs to be added). - Tuned parallelization to better fit hardware since otherwise a lot of strain is put on the OS's scheduler with the script's coroutine behaviors. - Sped up some of the slower parts (JSON to proto conversion) of the script by better utilizing parallel coroutines. - Added support for batch version fetching from remote, though it's currently disabled since there's an issue with using this: oppia/oppia#18241. - Silence illegal reflection access warning caused by Retrofit and made other quality-of-life improvements to script output. There's still more analysis required before this script can be considered done from an alpha perspective, including: - Double-checking constraint checking is working as expected (current reasoning suggests the language comprehension check isn't working). - Adding legacy proto conversion & output support. - Verifying that all failing responses from the emulated server are correctly failing. - Verifying that the structures and images import correctly in the app.
This commit: - Enables the language picker (being completed in #4762). - Bumps version codes (x2 since an extra re-release of 0.10 was necessary, and the updated version codes for that release weren't checked in).
These strings come from learner studies as a backup to logging events to Firebase. Local decoding is necessary to inspect and investigate the logs locally.
Relevant link: https://translatewiki.net/w/i.php?title=Special:ExportTranslations&language=pcm&group=oppia-android-app. This fixes some of the spacing issues that were introduced by during translation of the app strings.
This also adds support for Arabic in production builds (along with Nigerian Pidgin), and fixes some issues with the Swahili configuration. Nigeria was added as a new region for languages, including for production.
These changes aren't release blockers but they've been irking me for a while, so it's nice to get this output nicer. :)
Specifically: - Follow-up fixes to add Naija support to the new language selector. - Two post-merge nit fixes for the new language selector. - Fixed locale selection by adjusting the matching algorithm and also moving the pcm translations to be NG-specific. Some UI tests were removed that are particularly difficult to make pass, and don't add a lot of value.
This enables spotlights specifically for alpha builds (via a new alpha-only platform parameter module). This commit also adjusts spotlights' overlay background color since it's a bit too dark as-is. A new counter was added to count events during the app being open to help track for lost events between two events that are received.
Specifically: - Addressed failing static checks (including adding one test exemption for the new alpha-specific platform parameters module). - Fixed lint issues. - Added tests for the new debug event sequence number (and fixed existing tests for it). - Added some notes in languages.proto on creole languages. - Updated the profile selection logic in AndroidLocaleProfileFactory to more correctly prioritize Android language codes. This fixes language selection more in-line with the existing infrastructure than the previous solution, however it will require a large set of changes to AndroidLocaleProfileFactoryTest (which will happen in a follow-up commit). - Removed all temporary TODOs that are either addressed, or are being tracked elsewhere. There are still some items that need to be addressed yet. Note that Nigerian Pidgin has not yet been tested with content. Any fixes necessary for that will be brought in via a follow-up commit.
As part of this, a few small issues were fixed in AndroidLocaleFactory and its implementation was essentially recreated from the ground up to better leverage code reuse (and thus gain more confidence in test breadth since the actual contract of this factory is quite complex to test comprehensively).
This removes some no-longer-needed comments from AndroidLocaleFactory. It also updates the factory to be a bit more robust against missing region definitions (rather than trying to proceed which could result in an exception being thrown when it doesn't need to be). This also fixes InitializeDefaultLocaleRule incorrectly creating an empty region in cases when no region data is provided. This change fixes the StateFragmentLocalTest failures reported by CI.
For some reason, fixing these 3 tests as part of the 5 that were failing before is causing them to hang on Gradle (probably because they're actually working correctly now, though it's not clear why it's hanging vs. failing). Rather than investigating a potentially hard-to-track-down issue, this just disables the tests in Gradle since Gradle is going away soon, anyway, and the tests already run & pass with Bazel.
Some: - Using an android_binary now so a deployable version of it can be built. - Added a BUNCH of failure tolerance in multiple areas as part of getting things working "well enough" for the upcoming beta launch. - Added a BUNCH of new analysis and metrics. Some of it's wrong, but a lot of it has correctly identified problems. - Added conversion to legacy proto. - Lifted the overwriting error for output files to make things a bit more streamlined.
Specifically: - Disable Swahili again since it's not actually read for production launch. - Fix audio language localization during audio selection. - Fix a one-off crash that I noticed while changing languages and navigating. - Added a built-in recovery mode to the decode event string utility to ensure that even truncated strings can at least be partially recovered.
BenHenning
changed the title
Conflicts: app/src/main/java/org/oppia/android/app/translation/BUILD.bazel third_party/maven_install.json
|
Hi @BenHenning, I'm going to mark this PR as stale because it hasn't had any updates for 7 days. If no further activity occurs within 7 days, it will be automatically closed so that others can take up the issue. |
oppiabot
bot
added
the
stale
Specifically - Fix text reference tracking for invalid tags. - Remove unused exploration container prop that was removed server-side. - Add automatic gif->png conversion for textproto v1 (will need to be verified as working when the next release assets are downloaded). And, it seems like it might not be 100% working quite yet.
oppiabot
bot
removed
the
stale
It wasn't working well, and the better fix is to not use Gifs in lessons (since there is no immediate plan to support animations, and the app won't handle Gif animations out-of-the-box currently, anyway).
|
Hi @BenHenning, I'm going to mark this PR as stale because it hasn't had any updates for 7 days. If no further activity occurs within 7 days, it will be automatically closed so that others can take up the issue. |
oppiabot
bot
added
the
stale
oppiabot
bot
removed
the
stale
Expose download_lessons for external use (so that it can be used outside this branch while the branch continues to be developed).
This reverts some of the newer Kotlin functionality used in the script so that it can be built on current Android develop. It can be reverted once mainline is updated to Kotlin 1.5+.
Previous approach wasn't very compatible with Bazel rules, plus it's a bit confusing from a user perspective.
This is only needed since the codegen doesn't seem to be working entirely with Moshi 1.11 (which is what mainline is using, and it can't upgrade to 1.13 without first upgrading Kotlin).
See previous commit for reasoning.
Add support for pinning topic list versions to allow for deterministic asset downloads.
|
Hi @BenHenning, I'm going to mark this PR as stale because it hasn't had any updates for 7 days. If no further activity occurs within 7 days, it will be automatically closed so that others can take up the issue. |
oppiabot
bot
added
the
stale
adhiamboperes
removed
the
stale
|
Hi @BenHenning, I'm going to mark this PR as stale because it hasn't had any updates for 7 days. If no further activity occurs within 7 days, it will be automatically closed so that others can take up the issue. |
oppiabot
bot
added
the
stale
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Explanation
Fixes #1547
TODO: Finish.
Essential Checklist
For UI-specific PRs only
If your PR includes UI-related changes, then: