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

fix chess examples by using dlt rest client #2375

Open
wants to merge 1 commit into
base: devel
Choose a base branch
from

Conversation

mattseddon
Copy link

Description

This PR fixes the errors in the chess examples from docs/examples.

Here is a recent CI failure: https://github.com/dlt-hub/dlt/actions/runs/13662290829/job/38196000155

Additional Context

I have been poking around the repository and looking at bugs. I noticed that these examples are currently failing in the CI. The underlying issue is that the API is returning 404s for some of the requests made. If there is a better way to fix the examples then please LMK. I could not find an obvious way to add validation (i.e. if a player does not exist ignore them) in the docs.

Copy link

netlify bot commented Mar 5, 2025

Deploy Preview for dlt-hub-docs canceled.

Name Link
🔨 Latest commit a575e53
🔍 Latest deploy log https://app.netlify.com/sites/dlt-hub-docs/deploys/67ca3a995cf46c0009faa831

@mattseddon mattseddon marked this pull request as ready for review March 5, 2025 01:35
@mattseddon mattseddon force-pushed the fix/chess-examples branch from 4064680 to 6e794b0 Compare March 5, 2025 02:28
@sh-rp sh-rp added the ci from fork run ci workflows on a pr even if they are from a fork label Mar 5, 2025
@sh-rp
Copy link
Collaborator

sh-rp commented Mar 6, 2025

Hey @mattseddon, thanks your PR, much appreciated! Could you rebase your fork to the latest devel on this repo? The tests are not executing and I think the fix for this should be on our current devel branch.

@mattseddon mattseddon force-pushed the fix/chess-examples branch from 6e794b0 to bade748 Compare March 6, 2025 08:42
@mattseddon
Copy link
Author

Hey @mattseddon, thanks your PR, much appreciated! Could you rebase your fork to the latest devel on this repo? The tests are not executing and I think the fix for this should be on our current devel branch.

@sh-rp I'd assume that the examples tests are not running because they're all marked with the following:

@skipifgithubfork
@pytest.mark.forked

Unless something has changed?

Also, it looks like this change only addresses a transient issue with the chess.com API (the pipeline seems to be green again).

I think the reason that the change worked was that the RESTClient instantiates a new Client with raise_for_status=False. I tested by running the data into duckdb. When the API was misbehaving I got data like this:

image

@mattseddon mattseddon force-pushed the fix/chess-examples branch from bade748 to a575e53 Compare March 7, 2025 00:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci from fork run ci workflows on a pr even if they are from a fork
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants