feat: Move SQLAlchemy url reference to config #13182
Conversation
| const DEFAULT_TAB_KEY = '1'; | ||
| const DEFAULT_SQLALCHEMY_DOCS_URL = | ||
| 'https://docs.sqlalchemy.org/en/rel_1_2/core/engines.html#'; |
There was a problem hiding this comment.
do you want to put this in the default config file instead?
There was a problem hiding this comment.
Actually I see you also have it there.. seems like unless someone overwrites it with an empty string in their own config, it should always exist and we shouldn't need a default here as well?
There was a problem hiding this comment.
The default i'm using since the unit test aren't playing nice, i couldn't figure out how to get the wrappingComponent must render its children!
| @@ -132,6 +145,10 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({ | |||
| const [db, setDB] = useState<DatabaseObject | null>(null); | |||
| const [isHidden, setIsHidden] = useState<boolean>(true); | |||
| const [tabKey, setTabKey] = useState<string>(DEFAULT_TAB_KEY); | |||
| const conf = useSelector((state: RootState) => { | |||
| console.log(state); | |||
superset/config.py
Outdated
| @@ -1092,6 +1092,9 @@ class CeleryConfig: # pylint: disable=too-few-public-methods | |||
| # It will get executed each time when user open a chart's explore view. | |||
| DATASET_HEALTH_CHECK = None | |||
|
|
|||
| # SQLAlchema link doc reference | |||
| @@ -402,7 +419,7 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({ | |||
| <div className="helper"> | |||
| {t('Refer to the ')} | |||
| <a | |||
| href="https://docs.sqlalchemy.org/en/rel_1_2/core/engines.html#" | |||
| href={conf?.SQLALCHEMY_DOCS_URL ?? DEFAULT_SQLALCHEMY_DOCS_URL} | |||
There was a problem hiding this comment.
per above, it seems that just conf.SQLALCHEMY_DOCS_URL should always have a value.
There was a problem hiding this comment.
do we want to also update the string text below?
There was a problem hiding this comment.
what string text are your referencing?
There was a problem hiding this comment.
the text for the link. "refer to the sqlalchemy docs...". It's in the ticket, too.
superset/config.py
Outdated
| @@ -1092,6 +1092,9 @@ class CeleryConfig: # pylint: disable=too-few-public-methods | |||
| # It will get executed each time when user open a chart's explore view. | |||
| DATASET_HEALTH_CHECK = None | |||
|
|
|||
| # SQLAlchema link doc reference | |||
| SQLALCHEMY_DOCS_URL = "https://docs.sqlalchemy.org/en/rel_1_2/core/engines.html#" | |||
There was a problem hiding this comment.
tiny tiny nit, but prob don't need the # at the end.
I suppose we should also update the link to https://docs.sqlalchemy.org/en/13/core/engines.html since we're on 1.3 now.
Codecov Report
@@ Coverage Diff @@
## master #13182 +/- ##
===========================================
+ Coverage 53.06% 65.00% +11.94%
===========================================
Files 489 1043 +554
Lines 17314 49394 +32080
Branches 4482 5338 +856
===========================================
+ Hits 9187 32109 +22922
- Misses 8127 17069 +8942
- Partials 0 216 +216
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
| @@ -1092,6 +1092,9 @@ class CeleryConfig: # pylint: disable=too-few-public-methods | |||
| # It will get executed each time when user open a chart's explore view. | |||
| DATASET_HEALTH_CHECK = None | |||
|
|
|||
| # SQLalchemy link doc reference | |||
| SQLALCHEMY_DOCS_URL = "https://docs.sqlalchemy.org/en/13/core/engines.html" | |||
Manual conflict resolution (cherry picked from commit 3c58fc5)
mistercrunch
added
🏷️ bot
SUMMARY
Giving the ability for us at preset, to set the value for the sqlalchemy doc when running in our env
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TEST PLAN
ADDITIONAL INFORMATION