You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2021/09/30 18:37:16 UTC
[GitHub] [superset] Lawful2002 opened a new pull request #16926: [WIP] fix: Read MAPBOX_API_KEY from environment):
Lawful2002 opened a new pull request #16926:
URL: https://github.com/apache/superset/pull/16926
<!---
Please write the PR title following the conventions at https://www.conventionalcommits.org/en/v1.0.0/
Example:
fix(dashboard): load charts correctly
-->
### SUMMARY
<!--- Describe the change below, including rationale and design decisions -->
This PR fixes: #16781
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<!--- Skip this if not applicable -->
### TESTING INSTRUCTIONS
<!--- Required! What steps can be taken to manually verify the changes? -->
1. Added `MAPBOX_API_KEY = get_env_variable("MAPBOX_API_KEY")` in `superset_config.py`
2. added `MAPBOX_API_KEY=''` in `docker/.env` and `docker/.env-non-dev`
3. Updated docs at: https://github.com/apache/superset/blob/a330b664c13e34a4c7ca954f9a3a527a43a5535f/docs/src/pages/docs/frequently-asked-questions-page.mdx#why-is-the-map-not-visible-in-the-geospatial-visualization
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [x] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] 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
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] Lawful2002 commented on a change in pull request #16926: fix: Read MAPBOX_API_KEY from environment
Posted by GitBox <gi...@apache.org>.
Lawful2002 commented on a change in pull request #16926:
URL: https://github.com/apache/superset/pull/16926#discussion_r773907259
##########
File path: docker/pythonpath_dev/superset_config.py
##########
@@ -44,7 +44,7 @@ def get_env_variable(var_name: str, default: Optional[str] = None) -> str:
)
raise EnvironmentError(error_msg)
-
+MAPBOX_API_KEY = get_env_variable("MAPBOX_API_KEY")
Review comment:
I'm not really sure if this duplication is required or not, if I'm being honest. However as the API key is already getting fetched in [https://github.com/apache/superset/blob/master/superset/config.py#L695](https://github.com/apache/superset/blob/master/superset/config.py#L695), this duplication may not be required. That said though, the linked issue (https://github.com/apache/superset/issues/16781) asked `MAPBOX_API_KEY = get_env_variable("MAPBOX_API_KEY")` to be added to `docker/pythonpath_dev/superset_config.py`. Also the documentation at https://github.com/apache/superset/blob/a330b664c13e34a4c7ca954f9a3a527a43a5535f/docs/src/pages/docs/frequently-asked-questions-page.mdx#why-is-the-map-not-visible-in-the-geospatial-visualization wants that key in that particular file/location.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] Lawful2002 commented on pull request #16926: fix: Read MAPBOX_API_KEY from environment
Posted by GitBox <gi...@apache.org>.
Lawful2002 commented on pull request #16926:
URL: https://github.com/apache/superset/pull/16926#issuecomment-1000205244
> Hey @Lawful2002 , there's only black formatting to fix and this PR can be merged as it's approved. It would be super useful to have it :)
I've fixed the formatting. Sorry for the delay
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] jakubczaplicki commented on pull request #16926: fix: Read MAPBOX_API_KEY from environment
Posted by GitBox <gi...@apache.org>.
jakubczaplicki commented on pull request #16926:
URL: https://github.com/apache/superset/pull/16926#issuecomment-999566589
Hey @Lawful2002 , there's only black formatting to fix and this PR can be merged as it's approved. It would be super useful to have it :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] villebro commented on pull request #16926: fix: Read MAPBOX_API_KEY from environment
Posted by GitBox <gi...@apache.org>.
villebro commented on pull request #16926:
URL: https://github.com/apache/superset/pull/16926#issuecomment-966091139
@Lawful2002 I closed/reopened the PR, now CI checks seem to have started correctly, sorry for the delay. Will take a look once tests complete.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] Lawful2002 commented on pull request #16926: fix: Read MAPBOX_API_KEY from environment
Posted by GitBox <gi...@apache.org>.
Lawful2002 commented on pull request #16926:
URL: https://github.com/apache/superset/pull/16926#issuecomment-941894552
@junlincc Hey, the workflow tests haven't started on their own in a while. Would it be possible to manually start these tests?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] Lawful2002 commented on pull request #16926: fix: Read MAPBOX_API_KEY from environment
Posted by GitBox <gi...@apache.org>.
Lawful2002 commented on pull request #16926:
URL: https://github.com/apache/superset/pull/16926#issuecomment-941894552
@junlincc Hey, the workflow tests haven't started on their own in a while. Would it be possible to manually start these tests?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] jakubczaplicki commented on pull request #16926: fix: Read MAPBOX_API_KEY from environment
Posted by GitBox <gi...@apache.org>.
jakubczaplicki commented on pull request #16926:
URL: https://github.com/apache/superset/pull/16926#issuecomment-1003951298
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] villebro closed pull request #16926: fix: Read MAPBOX_API_KEY from environment
Posted by GitBox <gi...@apache.org>.
villebro closed pull request #16926:
URL: https://github.com/apache/superset/pull/16926
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] Lawful2002 closed pull request #16926: fix: Read MAPBOX_API_KEY from environment
Posted by GitBox <gi...@apache.org>.
Lawful2002 closed pull request #16926:
URL: https://github.com/apache/superset/pull/16926
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] hbruch commented on a change in pull request #16926: fix: Read MAPBOX_API_KEY from environment
Posted by GitBox <gi...@apache.org>.
hbruch commented on a change in pull request #16926:
URL: https://github.com/apache/superset/pull/16926#discussion_r773881434
##########
File path: docker/pythonpath_dev/superset_config.py
##########
@@ -44,7 +44,7 @@ def get_env_variable(var_name: str, default: Optional[str] = None) -> str:
)
raise EnvironmentError(error_msg)
-
+MAPBOX_API_KEY = get_env_variable("MAPBOX_API_KEY")
Review comment:
Is this duplication of https://github.com/apache/superset/blob/master/superset/config.py#L695 required?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] Lawful2002 commented on a change in pull request #16926: fix: Read MAPBOX_API_KEY from environment
Posted by GitBox <gi...@apache.org>.
Lawful2002 commented on a change in pull request #16926:
URL: https://github.com/apache/superset/pull/16926#discussion_r773907259
##########
File path: docker/pythonpath_dev/superset_config.py
##########
@@ -44,7 +44,7 @@ def get_env_variable(var_name: str, default: Optional[str] = None) -> str:
)
raise EnvironmentError(error_msg)
-
+MAPBOX_API_KEY = get_env_variable("MAPBOX_API_KEY")
Review comment:
I'm not really sure if this duplication is required or not, if I'm being honest. However as the API key is already getting fetched in [https://github.com/apache/superset/blob/master/superset/config.py#L695](https://github.com/apache/superset/blob/master/superset/config.py#L695), this duplication may not be required. That said though, the linked issue (https://github.com/apache/superset/issues/16781) asked `MAPBOX_API_KEY = get_env_variable("MAPBOX_API_KEY")` to be added to `docker/pythonpath_dev/superset_config.py`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org