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

Improves setUp by using the test_config parameter #38

Merged
merged 1 commit into from
Feb 29, 2024

Conversation

CurtesMalteser
Copy link
Contributor

This improvement avoids create two database instances in Flask which is not allowed on newer version.
By leveraging on already existing code, I propose the following in this commit.

  • Passes the self.database_path to create_app
  • setup_db takes the database_path as argument from the test_config
  • setup_db is only called once and is compatible with newer versions of Flask if the project dependencies are updated

@CurtesMalteser CurtesMalteser requested a review from a team as a code owner February 23, 2024 18:29
@CurtesMalteser CurtesMalteser requested review from SudKul and removed request for a team February 23, 2024 18:29
@Mido055
Copy link

Mido055 commented Feb 27, 2024

Reviewed the setup_db enhancement. The dynamic database_path and single instance approach aligns well with Flask's best practices and increases compatibility with newer versions.

@SudKul SudKul merged commit 7d64e39 into udacity:main Feb 29, 2024
1 check failed
@SudKul
Copy link
Contributor

SudKul commented Feb 29, 2024

@sarah-udacity ^^

@CurtesMalteser CurtesMalteser deleted the feature/use_test_config branch February 29, 2024 15:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants