Skip to content

fix: loading examples in CI returns http error "too many requests" - #33412

Merged
mistercrunch merged 3 commits into
masterfrom
fix_example_load
May 13, 2025
Merged

mistercrunch merged 3 commits into
masterfrom
fix_example_load

Conversation

@mistercrunch

@mistercrunch mistercrunch commented May 12, 2025 •

Copy link
Copy Markdown
Member

Recently in #33354 I tried to address http 429 "too many attempts that we started getting in CI. This PR adds a bit of custom logic with retries and expodential backoffs to work around the issue. Ideally we'd have another approach using a proper CDN that supports our needs or use a github repo to clone locally, but this should help relieve the issue.

Regardless of the approach, having a central function examples.helpers.read_example_csv is a step in the right direction as it centralizes the logic used to load the csv into a single DRY place.

Recently in #33354 I tried to address http 429 "too many attempts that we started getting in CI. This PR adds a bit of custom logic with retries and expodential backoffs to work around the issue. Ideally we'd have another approach using a proper CDN that supports our needs or use a github repo to clone locally, but this should help relieve the issue.
@dosubot dosubot Bot added the doc:examples Related to example datasets and dashboards label May 12, 2025
@mistercrunch mistercrunch changed the title fix: loading examples in CI returns http error 429 fix(ci): loading examples in CI returns http error 429 May 12, 2025

@korbit-ai korbit-ai Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review by Korbit AI

Korbit automatically attempts to detect when you fix issues in new commits.
Category Issue Status
Functionality Incorrect Data Source for Airports ▹ view ✅ Fix detected
Performance Excessive Base Wait Time in Backoff Strategy ▹ view 🧠 Not in scope
Readability Fragmented string formatting in retry message ▹ view
Files scanned ​
File Path Reviewed
superset/examples/paris.py ✅
superset/examples/sf_population_polygons.py ✅
superset/examples/bart_lines.py ✅
superset/examples/random_time_series.py ✅
superset/examples/flights.py ✅
superset/examples/country_map.py ✅
superset/examples/energy.py ✅
superset/examples/long_lat.py ✅
superset/examples/multiformat_time_series.py ✅
superset/examples/helpers.py ✅
superset/examples/world_bank.py ✅
superset/examples/birth_names.py ✅

Explore our documentation to understand the languages and file types we support and the files we ignore.

Check out our docs on how you can make Korbit work best for you and your team.

Loving Korbit!? Share us on LinkedIn Reddit and X

def read_example_csv(
filepath: str,
max_attempts: int = 5,
wait_seconds: float = 60,

This comment was marked as resolved.

Comment on lines +156 to +160
print(
f"HTTP 429 received from {url}. ",
f"Retrying in {sleep_time:.1f}s ",
"(attempt {attempt}/{max_attempts})...",
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Fragmented string formatting in retry message category Readability

Tell me more
What is the issue?

Using concatenated print statements makes the retry message harder to read and maintain. The f-strings are split unnecessarily across multiple lines.

Why this matters

Split string formatting makes the message structure harder to follow and introduces potential formatting inconsistencies. Single f-string would be clearer.

Suggested change ∙ Feature Preview
print(f"HTTP 429 received from {url}. Retrying in {sleep_time:.1f}s (attempt {attempt}/{max_attempts})...")
Provide feedback to improve future suggestions

Nice Catch Incorrect Not in Scope Not in coding standard Other

💬 Looking for more details? Reply to this comment to chat with Korbit.

Comment thread superset/examples/flights.py Outdated
Comment on lines +45 to +47
airports = read_example_csv(
"flight_data.csv.gz", encoding="latin-1", compression="gzip"
)

This comment was marked as resolved.

@mistercrunch mistercrunch changed the title fix(ci): loading examples in CI returns http error 429 fix: loading examples in CI returns http error 'too many requests' May 12, 2025
@mistercrunch mistercrunch changed the title fix: loading examples in CI returns http error 'too many requests' fix:loading examples in CI returns http error 'too many requests' May 13, 2025
@mistercrunch mistercrunch changed the title fix:loading examples in CI returns http error 'too many requests' fix: loading examples in CI returns http error "too many requests" May 13, 2025
@mistercrunch
mistercrunch merged commit 7f14e43 into master May 13, 2025
@mistercrunch
mistercrunch deleted the fix_example_load branch May 13, 2025 15:36
@michael-s-molina

Copy link
Copy Markdown
Member

@mistercrunch Why do we need this follow-up? Didn't the previous one fix the issue? I'm just asking to know if I need to cherry-pick this into 5.0 like I did with the previous one.

@mistercrunch

Copy link
Copy Markdown
Member Author

seems it helped - went from seeing a lot of 429 in CI to much fewer, but there are still issues with it. Not sure if this fix fully fixes things either, working on yet another follow up.

LevisNgigi pushed a commit to LevisNgigi/superset that referenced this pull request Jun 18, 2025
qfcwell pushed a commit to qfcwell/superset that referenced this pull request May 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

doc:examples Related to example datasets and dashboards preset-io size/L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants