fix: catch error when masking encrypted extra is none

[eschutho]

SUMMARY

When masking/unmasking the encrypted extra field, this change allows imported dbs to have a blank encrypted extra field, because they are not imported.

TESTING INSTRUCTIONS

Import a database and try to edit it in the database connection modal.

ADDITIONAL INFORMATION

• Has associated issue:
• Required feature flags:
• Changes UI
• Includes DB Migration (follow approval process in SIP-59)
  • Migration is atomic, supports rollback & is backwards-compatible
  • Confirm DB migration upgrade and downgrade tested
  • Runtime estimates and downtime expectations provided
• Introduces new feature or API
• Removes existing feature or API

[eschutho]

@betodealmeida the other option here would be to have a default value if encrypted_extra is None, but I opted for including it in the try/catch since it already existed. Of course, this requires the type ignores because the linter isn't aware of the try/except.

[betodealmeida]

Sounds good! Alternatively I think this would also work:

def mask_encrypted_extra(cls, encrypted_extra: Optional[str]) -> Optional[str]:
    if encrypted_extra is None:
        return encrypted_extra

    try:
        ...

[eschutho]

Thanks for the suggestion. Yeah I think it looks better this way without the type ignores. I also left the TypeError in the except block just in case.

[codecov]

Codecov Report

Merging #21570 (f0556ce) into master (2224ebe) will decrease coverage by 11.37%.
The diff coverage is 81.81%.

❗ Current head f0556ce differs from pull request most recent head ad122fc. Consider uploading reports for the commit ad122fc to get more accurate results

@@             Coverage Diff             @@
##           master   #21570       +/-   ##
===========================================
- Coverage   66.67%   55.30%   -11.38%     
===========================================
  Files        1793     1794        +1     
  Lines       68523    68642      +119     
  Branches     7281     7281               
===========================================
- Hits        45688    37960     -7728     
- Misses      20971    28818     +7847     
  Partials     1864     1864               

Flag	Coverage Δ
hive	52.91% <40.90%> (-0.18%)	⬇️
mysql	?
postgres	?
presto	52.81% <40.90%> (-0.18%)	⬇️
python	57.90% <81.81%> (-23.51%)	⬇️
sqlite	?
unit	50.93% <81.81%> (-0.14%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
superset/models/core.py	79.24% <71.42%> (-10.97%)	⬇️
superset/db_engine_specs/bigquery.py	54.35% <75.00%> (-27.32%)	⬇️
superset/db_engine_specs/base.py	76.11% <100.00%> (-13.47%)	⬇️
superset/db_engine_specs/gsheets.py	77.16% <100.00%> (+1.76%)	⬆️
superset/utils/dashboard_import_export.py	0.00% <0.00%> (-100.00%)	⬇️
superset/key_value/commands/update.py	0.00% <0.00%> (-88.89%)	⬇️
superset/key_value/commands/delete.py	0.00% <0.00%> (-85.30%)	⬇️
superset/key_value/commands/delete_expired.py	0.00% <0.00%> (-80.77%)	⬇️
superset/dashboards/commands/importers/v0.py	15.62% <0.00%> (-76.25%)	⬇️
superset/datasets/commands/update.py	25.00% <0.00%> (-69.05%)	⬇️
... and 293 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[villebro]

A few code smell nits

[yousoph]

/testenv up

[github-actions]

@yousoph Ephemeral environment spinning up at http://35.91.149.182:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

[betodealmeida]

Thanks for catching and fixing this!

[betodealmeida]

Sounds good! Alternatively I think this would also work:

def mask_encrypted_extra(cls, encrypted_extra: Optional[str]) -> Optional[str]:
    if encrypted_extra is None:
        return encrypted_extra

    try:
        ...

[github-actions]

Ephemeral environment shutdown and build artifacts deleted.