-
Notifications
You must be signed in to change notification settings - Fork 5
Update dlt dependency from ^0.4.12 to ^1.11.0 #135
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
base: devel
Are you sure you want to change the base?
Conversation
92f696e
to
fcd89e6
Compare
- Upgraded dlt from ^0.4.12 to ^1.11.0 - Updated rest_api.py, utils.py, and other files - Cleanup and minor fixes# This is a combination of 13 commits. Update dlt dependency from ^0.4.12 to ^1.11.0 Updated 'update_rest_api.py' Updated Updated utils.py updated Updted Updated Updated Updated Updated Updated Updated updated pokemon Updated
fcd89e6
to
57152c8
Compare
REST_API_SOURCE_LOCATION = str(pathlib.Path(__file__).parent / "rest_api") | ||
|
||
# Placeholder for SecretsTomlConfig to resolve ImportError in tests | ||
SecretsTomlConfig = Dict[str, Any] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This does not seem to be used anywhere, does it? Why is it imported in the tests at all?
|
||
BASEPATH = "https://raw.githubusercontent.com/dlt-hub/verified-sources/master/sources/rest_api/" | ||
FILES = ["README.md", "__init__.py", "config_setup.py", "exceptions.py", "requirements.txt", "typing.py", "utils.py"] | ||
|
||
|
||
def update_rest_api(force: bool = False) -> None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can remove this and the commands that depend on it entirely
@@ -12,4 +12,4 @@ def test_pokemon_pipeline() -> None: | |||
|
|||
# there should be 20 entries with pokemon details loaded | |||
# (the id is only loaded on the full request) | |||
assert db.sql("SELECT count (*) from pokemon_data.pokemon as p WHERE p.id IS NOT NULL").fetchone()[0] == 40 | |||
assert db.sql("SELECT count (*) from pokemon_data.pokemon as p WHERE p.id IS NOT NULL").fetchone()[0] == 2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do you know why this has changed?
from dlt_init_openapi.config import REST_API_SOURCE_LOCATION, Config | ||
from rest_api.typing import EndpointResource, RESTAPIConfig | ||
from dlt_init_openapi.config import REST_API_SOURCE_LOCATION, Config, SecretsTomlConfig | ||
from dlt_init_openapi.parser.context import OpenapiContext |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is this line here? OpenapiContext does not seem to be used?
REST_API_SOURCE_LOCATION = str(pathlib.Path(__file__).parent.resolve() / "../rest_api") | ||
# In dlt>=1.11.0, the rest_api is part of the main package | ||
# For backwards compatibility, we keep a stub directory | ||
REST_API_SOURCE_LOCATION = str(pathlib.Path(__file__).parent / "rest_api") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this is needed anymore.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No description provided.