Jinso o fix/cors playground #2995
Conversation
Replace REST demo with DummyJSON users (CORS-safe for Playground)
- Switch from dummyjson to JSONPlaceholder users API - Add dev_mode=True for better development experience - Fix function parameter (remove unused dlt parameter) - Update table references from 'items' to 'users' consistently - Fix test assertion to expect 10 users instead of 50 - Add write_disposition='replace' to prevent data duplication - Improve data display with proper return statements
- Changed from users API (jsonplaceholder) to customers API (jaffle-shop) - Updated resource name from 'users' to 'customers' - Updated pipeline name from 'users_pipeline' to 'customers_pipeline' - Updated all table references and test assertions - Improved response handling with proper yield from pattern
✅ Deploy Preview for dlt-hub-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
- Resolved conflicts in playground.py - Kept customers pipeline implementation over users pipeline - Maintained all customers-related changes (API endpoint, resource name, table references) - Updated with latest upstream changes
- Fixed unused marimo import (auto-removed by ruff) - Fixed unused variable 'con' by returning it from connect function - Applied proper code formatting with ruff - All tests pass locally
| @dlt.resource(name="customers") | ||
| def fetch_customers(): | ||
| response = requests.get("https://jaffle-shop.dlthub.com/api/v1/customers") | ||
| yield from response.json() |
There was a problem hiding this comment.
| yield from response.json() | |
| yield response.json() |
| # NOTE: This line allows your data to be explored in the marimo datasources which is the third item from the top in the left sidebar | ||
| con = pipeline.dataset().ibis() | ||
| return | ||
| return (con,) |
There was a problem hiding this comment.
why do you return (con,) here?
There was a problem hiding this comment.
I think marimo adds this if you are in edit mode
There was a problem hiding this comment.
@Jinso-o just double check if it's not breaking anything
There was a problem hiding this comment.
It's basically marimo's way of saying "make this variable available to other cells and the UI"!
| def tests(pipeline): | ||
| # NOTE: this cell is only needed for testing this notebook on ci | ||
| assert pipeline.dataset().users.df().shape[0] == 10 | ||
| assert pipeline.dataset().customers.df().shape[0] > 0 |
There was a problem hiding this comment.
how many customers this endpoint returns? can you use exact number here?
There was a problem hiding this comment.
affle-shop API (used in the playground) returns 100 customers. Therefore for the maintenance and sustainability I assumed it would be better to use >0 then being specific with == 100 as jaffle shop might be updated later. If you recommend me to be more specific I would then use range however would it be better to keep simplicity in the code?
There was a problem hiding this comment.
It returns 100 because of default page_size=100
https://jaffle-shop.dlthub.com/docs#/customers/get_customers_api_v1_customers_get
I also think we should use rest_api source, check the example here: https://www.notion.so/dlthub/OSS-One-Pager-benefit-to-user-version-1-25a9fb8e23cf800aa35acbe0a28b995d?source=copy_link#25a9fb8e23cf8054a7e9c4dda5044c64
There was a problem hiding this comment.
Should I comment the details? or should I leave it so that the playground users can play with it?
There was a problem hiding this comment.
it looks good, leave it
- Change 'yield from response.json()' to 'yield response.json()' as requested by reviewer - Add ?limit=100 parameter to API call for consistent results - Update assertion to expect exactly 100 customers (== 100) - Addresses feedback from AstrakhantsevaAA in PR dlt-hub#2995
3 participants
Description
Updated API to jaffelshop
Related Issues
Additional Context