Require the SQLAlchemy URI on the database model

[willbarrett]

CATEGORY

Choose one

• Bug Fix
• Enhancement (new features, refinement)
• Refactor
• Add tests
• Build / Development Environment
• Documentation

SUMMARY

Currently, attempting to create a database connection without a SQLAlchemy connection string defined will throw a server error (calling strip on NoneType). After the change, the system will reject empty SQLAlchemy connection strings correctly.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TEST PLAN

ADDITIONAL INFORMATION

• Has associated issue:
• Changes UI
• Requires DB Migration.
• Confirm DB Migration upgrade and downgrade tested.
• Introduces new feature or API
• Removes existing feature or API

REVIEWERS

@etr2460 @betodealmeida @mistercrunch

[codecov-io]

Codecov Report

Merging #8720 into master will decrease coverage by <.01%.
The diff coverage is n/a.

@@            Coverage Diff            @@
##           master   #8720      +/-   ##
=========================================
- Coverage    65.9%   65.9%   -0.01%     
=========================================
  Files         483     483              
  Lines       24163   24165       +2     
  Branches     2770    2770              
=========================================
  Hits        15925   15925              
- Misses       8060    8062       +2     
  Partials      178     178

Impacted Files	Coverage Δ
superset/models/core.py	83.09% <ø> (ø)	⬆️
superset/models/schedules.py	88.88% <0%> (-2.03%)	⬇️
superset/sql_lab.py	77.57% <0%> (-0.47%)	⬇️
superset/views/datasource.py	96.07% <0%> (-0.08%)	⬇️
superset/models/annotations.py	92% <0%> (ø)	⬆️
superset/jinja_context.py	74.73% <0%> (ø)	⬆️
superset/views/sql_lab.py	60.52% <0%> (ø)	⬆️
superset/translations/utils.py	87.5% <0%> (ø)	⬆️
superset/views/log/views.py	100% <0%> (ø)	⬆️
superset/sql_parse.py	96.12% <0%> (ø)	⬆️
... and 12 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6c130b3...1aa94dc. Read the comment docs.

[john-bodley]

@willbarrett you’ll have to add a Alembic migration as part of this PR since you’re updating the data model.

[willbarrett]

@john-bodley this change was to trigger a UI change in the FAB database UI. I don't feel strongly about adding a NOT NULL constraint at the database level. Do you think such a constraint would be valuable?

[etr2460]

I think we should keep the data model and the database in sync as much as possible here. Also because any generated migration will then try to add this restriction again

[willbarrett]

Works for me. I'll add the migration.