Re: [PR] fix(models.core:Database): Allocate a SQLA engine once per process per URL. [superset]
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]
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]
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]
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]
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]
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]
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