fix: Update the migration to make it compatible with postgresql #306
Conversation
|
Thanks for the pull request, @qasimgulzar! This repository is currently maintained by Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review. 🔘 Get product approvalIf you haven't already, check this list to see if your contribution needs to go through the product review process.
🔘 Provide contextTo help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:
🔘 Get a green buildIf one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green. DetailsWhere can I find more information?If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources: When can I expect my changes to be merged?Our goal is to get community contributions seen and reviewed as efficiently as possible. However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:
💡 As a result it may take up to several weeks or months to complete a review and merge your PR. |
openedx-webhooks
added
the
open-source-contribution
qasimgulzar
changed the title
qasimgulzar
force-pushed
the
qasim/fix/migrations
branch
from
b60a59e to
871551b
Compare
|
@ormsbee I added some guard rails here. |
qasimgulzar
force-pushed
the
qasim/fix/migrations
branch
2 times, most recently
from
f39408c to
2e7625a
Compare
qasimgulzar
force-pushed
the
qasim/fix/migrations
branch
from
b21c0a1 to
3ea36df
Compare
|
@ormsbee I added test matrix and migration validation under this PR. |
ormsbee
left a comment
ormsbee
left a comment
There was a problem hiding this comment.
Please also increment the patch version of edx-when here: https://github.com/openedx/edx-when/blob/master/edx_when/__init__.py
| def alter_rel_date_column(apps, schema_editor): | ||
| db_engine = connection.settings_dict['ENGINE'] | ||
| with connection.cursor() as cursor: | ||
| if 'postgres' in db_engine: | ||
| cursor.execute( | ||
| 'ALTER TABLE edx_when_datepolicy ' | ||
| 'ALTER COLUMN rel_date TYPE interval USING rel_date::interval;' | ||
| ) | ||
| elif 'mysql' in db_engine: | ||
| cursor.execute( | ||
| 'ALTER TABLE edx_when_datepolicy ' | ||
| 'MODIFY rel_date BIGINT;' | ||
| ) | ||
|
|
||
|
|
||
| def reverse_alter_rel_date_column(apps, schema_editor): | ||
| db_engine = connection.settings_dict['ENGINE'] | ||
| with connection.cursor() as cursor: | ||
| if 'postgres' in db_engine: | ||
| cursor.execute( | ||
| 'ALTER TABLE edx_when_datepolicy ' | ||
| 'ALTER COLUMN rel_date TYPE varchar USING rel_date::varchar;' | ||
| ) | ||
| elif 'mysql' in db_engine: | ||
| cursor.execute( | ||
| 'ALTER TABLE edx_when_datepolicy ' | ||
| 'MODIFY rel_date VARCHAR(255);' | ||
| ) |
There was a problem hiding this comment.
Can you please document in docstrings why these two functions are necessary at all? Won't Django's standard handling of DurationField create a bigint or interval as necessary?
qasimgulzar
force-pushed
the
qasim/fix/migrations
branch
from
a35e11d to
7fc5189
Compare
github-project-automation
bot
moved this from Ready for Review
to Done
in Contributions
3 participants
Description:
This pull request introduces several changes to improve database compatibility, enhance CI workflows, and update migrations for better schema handling. Key updates include adding support for MySQL and PostgreSQL in CI pipelines, modifying database-related settings to use environment variables, and introducing a migration script to handle database-specific column types.
CI Workflow Enhancements:
.github/workflows/ci.ymlto include MySQL and PostgreSQL services for testing, with health checks and connection settings. Added matrix testing for both databases..github/workflows/migrations-checker-postgresql.ymlto validate migrations on both MySQL and PostgreSQL. This ensures compatibility with both database engines.mysqlclientandpsycopg2-binary) and handle coverage file renaming and merging for database-specific coverage artifacts.Database Configuration Updates:
test_settings.pyto use environment variables for database configuration, enabling dynamic switching between SQLite, MySQL, and PostgreSQL.Migrations Improvements:
edx_when/migrations/0005_auto_20190911_1056.pyto include a custom migration script for altering therel_datecolumn in theedx_when_datepolicytable. The script ensures proper data types are used for PostgreSQL (interval) and MySQL (BIGINT). [1] [2]Miscellaneous:
.coveragercto refine the file omission pattern for migrations, ensuring consistent coverage reporting.edx_whenpackage from2.5.1to2.5.2.Testing instructions: