Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle formula extraction from TML models #1069

Merged

Conversation

mars-lan
Copy link
Contributor

🤔 Why?

ThoughtSpot recently introduced models in favor or Worksheets. This has caused the crawler to crash when extracting formula from models:

Traceback (most recent call last):
  File "/usr/local/lib/python3.9/site-packages/metaphor/common/runner.py", line 61, in run_connector
    entities = connector.run_async()
  File "/usr/local/lib/python3.9/site-packages/metaphor/common/base_extractor.py", line 39, in run_async
    return asyncio.run(self.extract())
  File "/usr/local/lib/python3.9/asyncio/runners.py", line 44, in run
    return loop.run_until_complete(main)
  File "/usr/local/lib/python3.9/asyncio/base_events.py", line 647, in run_until_complete
    return future.result()
  File "/usr/local/lib/python3.9/site-packages/metaphor/thought_spot/extractor.py", line 105, in extract
    self.fetch_virtual_views()
  File "/usr/local/lib/python3.9/site-packages/metaphor/thought_spot/extractor.py", line 137, in fetch_virtual_views
    self.populate_formula()
  File "/usr/local/lib/python3.9/site-packages/metaphor/thought_spot/extractor.py", line 298, in populate_formula
    if column.name in column_expr_map:

🤓 What?

  • Add formula extraction support for models returned from the TML export API.
  • Gracefully handle all unknown TML object types.

🧪 Tested?

Verified against a production instance.

☑️ Checks

  • My PR contains actual code changes, and I have updated the version number in pyproject.toml.

@mars-lan mars-lan requested a review from alyiwang January 22, 2025 22:15
Copy link

☂️ Python Coverage

current status: ✅

Overall Coverage

Lines Covered Coverage Threshold Status
13634 12201 89% 85% 🟢

New Files

No new covered files...

Modified Files

File Coverage Status
metaphor/thought_spot/extractor.py 90% 🟢
metaphor/thought_spot/models.py 98% 🟢
TOTAL 94% 🟢

updated for commit: a91caff by action🐍

Copy link

codecov bot commented Jan 22, 2025

Codecov Report

Attention: Patch coverage is 37.50000% with 5 lines in your changes missing coverage. Please review.

Project coverage is 89.48%. Comparing base (14eef78) to head (a91caff).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
metaphor/thought_spot/extractor.py 0.00% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1069      +/-   ##
==========================================
- Coverage   89.51%   89.48%   -0.03%     
==========================================
  Files         210      210              
  Lines       13627    13634       +7     
==========================================
+ Hits        12198    12201       +3     
- Misses       1429     1433       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@alyiwang alyiwang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mars-lan mars-lan merged commit e50148e into main Jan 22, 2025
4 of 6 checks passed
@mars-lan mars-lan deleted the marslan/sc-30122/thoughtspot-crawler-crashing-due-to-missing branch January 22, 2025 22:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants