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 2022/01/25 22:59:33 UTC

[GitHub] [superset] betodealmeida commented on a change in pull request #18157: fix: remove standalone

betodealmeida commented on a change in pull request #18157:
URL: https://github.com/apache/superset/pull/18157#discussion_r792184387



##########
File path: superset/utils/urls.py
##########
@@ -33,3 +33,12 @@ def headless_url(path: str, user_friendly: bool = False) -> str:
 def get_url_path(view: str, user_friendly: bool = False, **kwargs: Any) -> str:
     with current_app.test_request_context():
         return headless_url(url_for(view, **kwargs), user_friendly=user_friendly)
+
+
+def get_screenshot_explorelink(url: Optional[str]) -> Optional[str]:

Review comment:
       Can you add a docstring to the function explaining what it does? As for types, it's better to have the signature be `(url: str) -> str:`, otherwise you need to handle the case where `url` is `None`.
   
   One thing I was thinking is that since this function is currently very specific we could make it a bit more generic, so it has utility outside of your use case. Eg:
   
   ```python
   def modify_url_query(url: str, **kwargs: Any) -> str:
       """
       Replace or add parameters to a URL.
       """
       parts = list(urllib.parse.urlsplit(url))
       params = urllib.parse.parse_qs(parts[3])
       for k, v in kwargs.items():
           if not isinstance(v, list):
               v = [v]
           params[k] = v
       
       parts[3] = "&".join(f"{k}={urllib.parse.quote(v[0])}" for k, v in params.items())
       return urllib.parse.urlunsplit(parts)
   ```
   
   Then you can call it for your use case as:
   
   ```python
   url = modify_url_query(self._content.url, standalone='0')
   ```




-- 
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