feat!: s/price_excl_tax/price for django oscar 3.1 upgrade #4291
Conversation
The Django Oscar 3.1 upgrade renames table partner_stockrecord.price_excl_tax to partner_stockrecord.price: https://github.com/openedx/ecommerce/blob/8cd7c39689954a16694d89eefe850fc6bca2462a/ecommerce/extensions/partner/migrations/0019_auto_20231108_1355.py#L44-L48 This commit mirrors the rename in course-discovery. Without this rename, I get on devstack: 2024-03-15 18:56:57,271 ERROR 200 [course_discovery.apps.course_metadata.management.commands.refresh_course_metadata] /edx/app/discovery/discovery/course_discovery/apps/course_metadata/management/commands/refresh_course_metadata.py:40 - EcommerceApiDataLoader failed! Traceback (most recent call last): File "/edx/app/discovery/discovery/course_discovery/apps/course_metadata/management/commands/refresh_course_metadata.py", line 37, in execute_loader run_loader() File "/edx/app/discovery/venvs/discovery/lib/python3.8/site-packages/backoff/_sync.py", line 105, in retry ret = target(*args, **kwargs) File "/edx/app/discovery/discovery/course_discovery/apps/course_metadata/management/commands/refresh_course_metadata.py", line 34, in run_loader return loader_class(*loader_args).ingest() File "/edx/app/discovery/discovery/course_discovery/apps/course_metadata/data_loaders/api.py", line 348, in ingest self._load_ecommerce_data() File "/edx/app/discovery/discovery/course_discovery/apps/course_metadata/data_loaders/api.py", line 370, in _load_ecommerce_data self._process_course_runs(course_runs) File "/edx/app/discovery/discovery/course_discovery/apps/course_metadata/data_loaders/api.py", line 488, in _process_course_runs self.update_seats(body) File "/edx/app/discovery/discovery/course_discovery/apps/course_metadata/data_loaders/api.py", line 553, in update_seats self.update_seat(course_run, product_body) File "/edx/app/discovery/discovery/course_discovery/apps/course_metadata/data_loaders/api.py", line 575, in update_seat price = Decimal(stock_record['price_excl_tax']) KeyError: 'price_excl_tax' CommandError: One or more of the data loaders above failed.
|
|
pshiu
changed the title
|
Closing in favor of alternative solution using: |
|
@pshiu Hi, your proposed solution in the last comment doesn't seem to be implemented in redwood or master branches for the data loader. So, the data loader is still failing with the 'price_excl_tax' |
|
@AfaqShuaib09 Hi, as @pshiu doesn't seem to have worked on this repo in a while, would you be able to answer the above comment? |
|
Hey @tecoholic, I'm not sure why this change didn't merge. |
|
Upon further review, it appears that the Django Oscar upgrade change has been merged into the master and 2u/main branches (openedx/ecommerce#4050). However, it was later reverted in the 2u/main branch, while the changes remain in the master branch. |
|
@AfaqShuaib09 Hi, thank you for the clarifications. I have created a PR for the redwood branch. Can you kindly review and merge this? #4450 |
Description
The Django Oscar 3.1 upgrade renames column
partner_stockrecord.price_excl_taxtopartner_stockrecord.price. [migration in openedx/ecommerce]This PR mirrors the rename in course-discovery.
Deployment Instructions
Additional Information
Testing Instructions
Error if not present on devstack
Without this rename, I get
KeyError: 'price_excl_tax'on devstack when trying to run therefresh_course_metadatamanagement command:Successful run of refresh_course_metadata if present on devstack
The command succeeds after this commit: