Re: [PR] fix(models.core:Database): Allocate a SQLA engine once per process per URL. [superset]

2024-07-08 Thread via GitHub


villebro commented on PR #27899:
URL: https://github.com/apache/superset/pull/27899#issuecomment-2213506605

   > From the failure code, I'm under the impression that the test is expecting 
result code 202 [ which would mean that the job has been submitted to a cellery 
worker ? ] and is instead getting a 200 [ meaning the result is being served 
from cache ? ]
   
   @pedro-r-marques the particular test is hopelessly flaky (we will likely 
need to remove it, as fixing it has proved to be difficult for the MySQL 
integration test). Can you rebase the PR to fix the conflict?


-- 
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-unsubscr...@superset.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For additional commands, e-mail: notifications-h...@superset.apache.org



Re: [PR] fix(models.core:Database): Allocate a SQLA engine once per process per URL. [superset]

2024-07-08 Thread via GitHub


ShubhamDalmia commented on PR #27899:
URL: https://github.com/apache/superset/pull/27899#issuecomment-2213133051

   Any updates on this?


-- 
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-unsubscr...@superset.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For additional commands, e-mail: notifications-h...@superset.apache.org



Re: [PR] fix(models.core:Database): Allocate a SQLA engine once per process per URL. [superset]

2024-04-12 Thread via GitHub


pedro-r-marques commented on PR #27899:
URL: https://github.com/apache/superset/pull/27899#issuecomment-2051780176

   @eschutho The formatting issue is fixed. Unfortunatly the test-mysql test 
seems to be failing in `test_chart_data_async`. I've attempted to run the test 
multiple times in my setup (using the same mysql version and redis cache 
configuration as the CI) with no luck.
   
   Any hints ? 
   
   From the failure code, I'm under the impression that the test is expecting 
result code 202 [ which would mean that the job has been submitted to a cellery 
worker ? ] and is instead getting a 200 [ meaning the result is being served 
from cache ? ]
   
   I've tried running the test both at the version of the code in the PR as 
well as at HEAD. It always passes for me. 


-- 
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-unsubscr...@superset.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For additional commands, e-mail: notifications-h...@superset.apache.org



Re: [PR] fix(models.core:Database): Allocate a SQLA engine once per process per URL. [superset]

2024-04-10 Thread via GitHub


eschutho commented on PR #27899:
URL: https://github.com/apache/superset/pull/27899#issuecomment-2048563314

   @pedro-r-marques I restarted CI and fixed the formatting issue. We'll see if 
it passes now. 


-- 
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-unsubscr...@superset.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For additional commands, e-mail: notifications-h...@superset.apache.org



Re: [PR] fix(models.core:Database): Allocate a SQLA engine once per process per URL. [superset]

2024-04-10 Thread via GitHub


eschutho commented on code in PR #27899:
URL: https://github.com/apache/superset/pull/27899#discussion_r1560137955


##
superset/models/core.py:
##
@@ -376,9 +407,7 @@ def get_effective_user(self, object_url: URL) -> str | None:
 return (
 username
 if (username := get_username())
-else object_url.username
-if self.impersonate_user
-else None
+else object_url.username if self.impersonate_user else None

Review Comment:
   Testing to see if this will pass CI



##
superset/models/core.py:
##
@@ -376,9 +407,7 @@ def get_effective_user(self, object_url: URL) -> str | None:
 return (
 username
 if (username := get_username())
-else object_url.username
-if self.impersonate_user
-else None
+else object_url.username if self.impersonate_user else None

Review Comment:
   Testing to see if this will fix CI



-- 
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-unsubscr...@superset.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For additional commands, e-mail: notifications-h...@superset.apache.org



Re: [PR] fix(models.core:Database): Allocate a SQLA engine once per process per URL. [superset]

2024-04-10 Thread via GitHub


eschutho commented on code in PR #27899:
URL: https://github.com/apache/superset/pull/27899#discussion_r1560137869


##
superset/models/core.py:
##
@@ -376,9 +407,7 @@ def get_effective_user(self, object_url: URL) -> str | None:
 return (
 username
 if (username := get_username())
-else object_url.username
-if self.impersonate_user
-else None
+else object_url.username if self.impersonate_user else None

Review Comment:
   ```suggestion
   else object_url.username
   if self.impersonate_user
   else None
   ```



-- 
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-unsubscr...@superset.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For additional commands, e-mail: notifications-h...@superset.apache.org



Re: [PR] fix(models.core:Database): Allocate a SQLA engine once per process per URL. [superset]

2024-04-08 Thread via GitHub


pedro-r-marques commented on PR #27899:
URL: https://github.com/apache/superset/pull/27899#issuecomment-2043065552

   The CI failures above:
   - Translations was a CI error.
   - pre-commit check wants to reformat lines of code in a file that I touched 
but have nothing to do with the PR. What is the policy for the project ? should 
I add the change to make 'black' happy ? Or ignore it since it is unrelated to 
the intended change ?


-- 
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-unsubscr...@superset.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For additional commands, e-mail: notifications-h...@superset.apache.org