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.

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 invocations of read_channel_metadata_from_db_file to expect a dict #8369

Merged
merged 1 commit into from
Sep 2, 2021

Conversation

rtibbles
Copy link
Member

Summary

References

Fixes #8251

Reviewer guidance

Can you now export to an external drive?


Testing checklist

  • Contributor has fully tested the PR manually
  • If there are any front-end changes, before/after screenshots are included
  • Critical user journeys are covered by Gherkin stories
  • Critical and brittle code paths are covered by unit tests

PR process

  • PR has the correct target branch and milestone
  • PR has 'needs review' or 'work-in-progress' label
  • If PR is ready for review, a reviewer has been added. (Don't use 'Assignees')
  • If this is an important user-facing change, PR or related issue has a 'changelog' label
  • If this includes an internal dependency change, a link to the diff is provided

Reviewer checklist

  • Automated test coverage is satisfactory
  • PR is fully functional
  • PR has been tested for accessibility regressions
  • External dependency files were updated if necessary (yarn and pip)
  • Documentation is updated
  • Contributor is in AUTHORS.md

@rtibbles rtibbles added the TODO: needs review Waiting for review label Aug 31, 2021
@rtibbles rtibbles added this to the 0.15.0 milestone Aug 31, 2021
@radinamatic
Copy link
Member

Export to an USB drive is now working again on Windows 10, although somewhat more slowly than I remember... 🤔
logs.zip

Win10 (start)  Running  - Oracle VM VirtualBox_025

@pcenov Could you confirm that exporting to external drive is working on Linux too, before we approve and merge? Thank you!

@pcenov
Copy link
Member

pcenov commented Sep 2, 2021

@radinamatic I confirm that export and import to external drive is now working in both Linux and Windows.

@radinamatic
Copy link
Member

Thank you, @pcenov! 👍🏽
I also managed to export from my dev VM, this should be good to go 💯

Copy link
Member

@radinamatic radinamatic left a comment

Choose a reason for hiding this comment

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

QA team says :shipit:

Comment on lines +124 to +134
"id": channel["id"],
"name": channel["name"],
"description": channel["description"],
"tagline": channel.get("tagline", ""),
"thumbnail": channel["thumbnail"],
"version": channel["version"],
"root": channel["root_id"],
"author": channel["author"],
"last_updated": channel.get("last_updated"),
"lang_code": channel.get("lang_code"),
"lang_name": channel.get("lang_name"),
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the inconsistency in getter style because some of these are supposed to throw value errors and others are supposed to return None if the items doesn't exist?

(Mainly just curious if this is part of an implicit required/optional API of some kind)

Copy link
Member Author

Choose a reason for hiding this comment

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

As it was 'like that when I got here' at this point, I am inferring - but I think these use of get versus direct key access is a result of newer fields being added that might be undefined in historic channel DB schema.

@rtibbles rtibbles merged commit 7252990 into learningequality:develop Sep 2, 2021
@rtibbles rtibbles deleted the channel_export_500 branch September 2, 2021 15:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
TODO: needs review Waiting for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants