fitzee opened a new pull request, #42427:
URL: https://github.com/apache/superset/pull/42427

   ### SUMMARY
   
   Reviewed production-safety follow-up for #42253.
   
   This replaces readiness and diagnostic selectors that rely on `data-test` 
attributes with the existing production classes 
(`.dashboard-component-chart-holder.dashboard-chart-id-*` and 
`.slice_container`). Production builds, including Docker builds under the 
production `BABEL_ENV`, run `babel-plugin-jsx-remove-data-test-id`, so the 
prior selectors became no-ops in the deployment mode that matters. The selector 
is shared across polling and diagnostics, and chart IDs are extracted from the 
unconditional `dashboard-chart-id-<digits>` class.
   
   The shared selector change also changes tiled semantics: it excludes 
Markdown and Dynamic components that share the holder test hook but are not 
charts. This fixes the latent #42119 Markdown/Dynamic `nothing_mounted` 
deadlock. Because #42119 has already merged with a selector that is a 
production no-op, that merged path still needs a follow-up using this 
production-safe selector.
   
   For non-tiled captures, dashboard/standalone captures use the holder 
predicate while `chart-container` captures use a positive chart predicate 
requiring a terminal marker and no `.loading`. Standalone dashboard captures 
warn when zero chart holders make the readiness gate inactive. The runtime 
guard derives an effective screenshot budget from Celery task hard/soft limits, 
reserving cleanup/error-transition time; this may duplicate or overlap the 
still-open #42118 and should be reconciled when that work lands.
   
   This hardens and fixes #42253. It does **not** establish the root cause of 
the pre-existing PointFive SC-113788 incident.
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   
   Not applicable; backend screenshot readiness and diagnostics only. The four 
frontend changes from the earlier patch are reverted because existing 
production classes are the contract.
   
   ### TESTING INSTRUCTIONS
   
   - `python3 -m pytest -q tests/unit_tests/utils/test_screenshot_utils.py 
tests/unit_tests/utils/webdriver_test.py` (requires the complete Superset 
Python test environment)
   - `ruff check superset/utils/screenshot_utils.py superset/utils/webdriver.py 
tests/unit_tests/utils/test_screenshot_utils.py 
tests/unit_tests/utils/webdriver_test.py`
   - `ruff format --check superset/utils/screenshot_utils.py 
superset/utils/webdriver.py tests/unit_tests/utils/test_screenshot_utils.py 
tests/unit_tests/utils/webdriver_test.py`
   - `pre-commit run --all-files` (mypy main and extension checks passed; local 
all-files run otherwise encountered environment/upstream blockers documented in 
the companion report)
   - `git diff --check`
   
   ### ADDITIONAL INFORMATION
   
   - [x] Has associated issue: fixes the #42253 production-safety gap and 
#42119 latent Markdown/Dynamic deadlock
   - [ ] 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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to