feat: refresh_xblock_skills management command - #129
Conversation
|
Thanks for the pull request, @navinkarkera! Please note that it may take us up to several weeks or months to complete a review and merge your PR. Feel free to add as much of the following information to the ticket as you can:
All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here. Please let us know once your PR is ready for our review and all tests are green. |
f6ebadc to
7c06170
Compare
kaustavb12
left a comment
There was a problem hiding this comment.
👍
Great job here @navinkarkera
I have left a few minor comments, otherwise this looks good to go from my end.
- I tested this: Tested the management command works as expected in devstack
- I read through the code
- I checked for accessibility issues
- Includes documentation
- I made sure any change in configuration variables is reflected in the corresponding client's
configuration-securerepository.
7c06170 to
1ec5d9c
Compare
|
@sameenfatima78 This is ready for your review. |
4b1dac0 to
2bdd5ff
Compare
2bdd5ff to
92e2ec8
Compare
|
@sameenfatima78 |
91ac383 to
cfcbeca
Compare
|
@sameenfatima78 Actually, we already merged another MR: #127 pointing to dev version of openedx-events and also the upstream MR is taking too long to be merged. So we can probably merge this as well and I'll create a new MR as soon as openedx-events is updated. Closed by mistake, re-opened |
chore: update openedx-events dep to dev commit
cfcbeca to
ec2a581
Compare
|
Hi @navinkarkera! Is this ready to be reviewed/merged? |
|
@mphilbrick211 Yes, thank you. |
|
@navinkarkera 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future. |
|
@navinkarkera Getting this error while publishing latest version to PyPI. https://github.com/openedx/taxonomy-connector/actions/runs/3799391856/jobs/6461895388 |
|
@sameenfatima78 Ohh, it seems we will have to wait until openedx/openedx-events#143 is merged and latest version is pushed to pypi. Apologies for this confusion as I was not aware of this restriction. |
@navinkarkera Yeah reverting it makes sense as it could block others from publishing a release. |
|
@sameenfatima78 Created #136 MR to revert changes depending on openedx/openedx-events#143. Please approve and merge. |

Adds command
refresh_xblock_skillsto update skills related to passed xblocks, or all xblocks in passed courses or all xblocks in all courses available fromget_course_metadata_provider.Reviewers:
Test instructions:
If you try out below commands in project directory, it should fail at ESME/lightcast API call due to incorrect URL and not before that. It should use test settings by default and use mock data for content. If not run
make testonce.To test
args-from-databaseargument open django shell and run below commands:Then run below command:
python manage.py refresh_xblock_skills --args-from-databaseAlthough, running above commands should cover the code in this MR, for end to end test with real metadata provider from course-discovery, I'll try to come up with test instructions once I figure out way to mock lightcast API or get free access to lightcast API
Merge checklist:
requirements/*.txtfiles)make upgrade && make requirementshave been run to regenerate requirements./manage.py makemigrationshas been runPost merge:
(so basically once your build finishes, after maybe a minute you should see the new version in PyPi automatically (on refresh))
make upgradein course-discovery will look for the latest version in PyPi.