-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
🎉 New Source: Weatherstack [python cdk] #16473
🎉 New Source: Weatherstack [python cdk] #16473
Conversation
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 will be reviewing this as part of the airbyte maintainer program, please bear with me as a run some checks on the changes.
Awesome, thank you! I also used @alafanechere 's work he did with the Openweather source as a guideline to make this source. In case you I would need another reviewer. |
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.
@robgleason good first stab at this PR, mostly cosmetic changes as per the review comments:
- Number of streams not matching up
- Lots of TODOs to be removed
- Spec needs to be updated with
order
airbyte_secret
pattern
and more notes - API queries notes not quite right, i.e.
query
can have not just cities - Broken logic in
source.py
where it will only work for NY or London - Various conditional checks and refactoring needed on
source.py
Additional to the review:
- You need to run
./gradlew format
- You need to add an entry to
airbyte-integrations/builds.md
airbyte-integrations/connectors/source-weatherstack/integration_tests/acceptance.py
Outdated
Show resolved
Hide resolved
airbyte-integrations/connectors/source-weatherstack/integration_tests/catalog.json
Outdated
Show resolved
Hide resolved
airbyte-integrations/connectors/source-weatherstack/unit_tests/test_incremental_streams.py
Outdated
Show resolved
Hide resolved
airbyte-integrations/connectors/source-weatherstack/unit_tests/test_source.py
Outdated
Show resolved
Hide resolved
airbyte-integrations/connectors/source-weatherstack/unit_tests/test_source.py
Outdated
Show resolved
Hide resolved
airbyte-integrations/connectors/source-weatherstack/source_weatherstack/source.py
Show resolved
Hide resolved
airbyte-integrations/connectors/source-weatherstack/source_weatherstack/source.py
Show resolved
Hide resolved
airbyte-integrations/connectors/source-weatherstack/source_weatherstack/source.py
Show resolved
Hide resolved
airbyte-integrations/connectors/source-weatherstack/source_weatherstack/spec.yaml
Outdated
Show resolved
Hide resolved
airbyte-integrations/connectors/source-weatherstack/source_weatherstack/source.py
Outdated
Show resolved
Hide resolved
…on/airbyte into New-Source-Weatherstack
I also did not use any Java in this connector, so I don't think I would need to use gradlew. |
@robgleason please run that command regarless if your connector is java or python. As you have not run this it will fail some lint checks. Shout if you have any trouble. The command needs to be run when you are in the root airbyte directory |
Yes, I have been having issue with running the command (./gradlew) in the root Airbyte directory. I have Docker running Airbyte with docker-compose up. The command (./gradlew) won't work because it is failing to start the Gradle Daemon and create the Java Virtual Machine. I haven't really started debugging that command yet. |
@robgleason I’ll help you on the format using gradle when you are done by raising a PR onto your local branch |
@koconder Cool, thanks. I’m going to look into my issue with gradle. I want to rewrite the source.py file. I should get the correct commits in within the next two days. I have some stuff outside of Github that I need to do. Also, I want to go into deeper as to why I based the check_connection on an array of just two cities. I will get to the bottom of this. |
@robgleason feel free to reach out when you are ready, as for the comments that are resolved feel free to mark as resolved. Ready to help when you are finished addressing the issues raised in the review. |
Hello @koconder , I remade the check_connection. I also reran all the integration tests and pasted the results on this PR. I also ran it in the production like environment and pasted images of the results being sent to a Postgres database and raw Airbyte data being displayed by Postgres in production like environment. Would you be able to run gradle for me? It appears that I would need to spend a bunch of time setting up the right Java environment. Also, you mentioned making my code more dry (using less code) by instead of using 4 seperate stream classes that I should just use one class. It looks like those classes are needed by the streams function which needs to return an array of 4 objects with particular class properties. The approach that I would take is create one class with functions like getEndpointAddress and pass correct endpoints to it. Would that be best way to have the goal to make it more dry? |
This reverts commit 1b40726.
@robgleason I’ll make the final changes as a PR to your branch along with gradle. Don't worry about making the connections DRY for now as this is an alpha connector. |
@koconder Awesome! I might even get the Github code pairing badge for this. Let me know if you need anything! |
@koconder The last issue with the catalogue seems to be coming from the "AssertionError: Record from historical stream should have some fields mentioned by json schema..." I added the json schemas into the catalogue with their corresponding stream. I believe this should fix it. I tested it with the Python scripts and they passed. Can you let me know how it works on gradle? |
@robgleason took another proper look at this, seems like the Free API is not supported for queries as the I'm testing now with a new API key that is a paid account to validate. Unit Test:
Source Acceptance Test:
Error:
|
Tests are passing using paid API, will see if i can help you rework otherwise will keep as-is and only support the paid API:
|
@robgleason i have added a toggle to free vs paid account to enable the historical stream, I have added conditions to the streams to support and now its all working as expected. Notes for Airbyte Team for CI Sandbox AccountFor Airbyte team FYI, the
Using a Paid subcription account:
Using a Free API Account:
|
/test connector=connectors/source-weatherstack
Build PassedTest summary info:
|
/publish connector=connectors/source-weatherstack
if you have connectors that successfully published but failed definition generation, follow step 4 here |
Awesome, cool! |
* Added Source Weatherstack * Clean Code. Took out TODO's. * Added link to get API key * Remade check_connection and added Constants * Added WeatherStack to source_definitions and updated Catalog.json * deleted configured_catalog.json (not in use) * Revert "deleted configured_catalog.json (not in use)" This reverts commit 1b40726. * chore: revert change * chore: format and lint * chore: attempt to patch unit tests * Added Classes for Unit tests * chore: fix coverage * chore: fix unit tests for weatherstream * chore: fix weather stack unittests * chore: fix unittests fr weatherstack * Added schemas to catalogue * Update setup.py * Update build.gradle * Delete sample_state.json * support for various account types * Update source.py * Delete .dccache * Update source_definitions.yaml * Update source_specs.yaml Co-authored-by: Vincent Koc <koconder@users.noreply.github.com> Co-authored-by: Vincent Koc <koconder@gmail.com> Co-authored-by: Vincent Koc <vincentkoc@ieee.org>
What
Made a Source for Weatherstack described in #7816 .
Weatherstack offers a free subscription plan for 250 requests per month when making an account. My API key still has 225 requests left and I can share it.
How
Followed documentation/tutorials to make source.
It passed the standard spec, check, discover, and read tests in Python:
Go to Weatherstack directory:
cd airbyte-integrations\connectors\source-weatherstack
Initialize environment:
python -m venv .venv
.venv\scripts\activate
pip install -r requirements.txt
Run Testing Scripts:
python main.py spec
python main.py check --config integration_tests/sample_config.json
python main.py check --config integration_tests/invalid_config.json
python main.py discover --config integration_tests/sample_config.json
python main.py read --config integration_tests/sample_config.json --catalog integration_tests/catalog.json
The API does not have swaggerui/openapi so I had to make the json schema files and configured_catalog.json. The user requested 4 streams, and the source.py file was modified to allow 4 streams.
Created a dev container to test source-
Recommended reading order
source.py
🚨 User Impact 🚨
No
Pre-merge Checklist
Expand the relevant checklist and delete the others.
New Connector
Community member or Airbyter
airbyte_secret
./gradlew :airbyte-integrations:connectors:<name>:integrationTest
.README.md
bootstrap.md
. See description and examplesdocs/integrations/<source or destination>/<name>.md
including changelog. See changelog exampledocs/integrations/README.md
airbyte-integrations/builds.md
Airbyter
If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.
/test connector=connectors/<name>
command is passing/publish
command described hereUpdating a connector
Community member or Airbyter
airbyte_secret
./gradlew :airbyte-integrations:connectors:<name>:integrationTest
.README.md
bootstrap.md
. See description and examplesdocs/integrations/<source or destination>/<name>.md
including changelog. See changelog exampleAirbyter
If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.
/test connector=connectors/<name>
command is passing/publish
command described hereConnector Generator
-scaffold
in their name) have been updated with the latest scaffold by running./gradlew :airbyte-integrations:connector-templates:generator:testScaffoldTemplates
then checking in your changesTests
Unit
Put your unit tests output here.
python main.py spec
{"type": "SPEC", "spec": {"documentationUrl": "https://docs.airbyte.io/integrations/sources/weatherstack", "connectionSpecification": {"$schema": "http://json-schema.org/draft-07/schema#", "title": "Weatherstack Spec", "type": "object", "required": ["access_key", "query", "historical_date"], "properties": {"access_key": {"order": 0, "type": "string", "description": "API access key used to retrieve data from the Weatherstack API.(https://weaype": "string", "description": "A location to query such as city, IP, latitudeLongitude, or zipcode. Multiple locations with semicolon seperated if using a professional plan or higher. For more info- (https://weatherstack.com/documentation#query_parameter)", "examples": ["New York", "London", "98101"]}, "historical_date": {"order": 2, "type": "string", "description":
MM-DD).", "examples": ["2015-01-21"], "pattern": "[0-9]{4}-[0-9]{2}-[0-9]{2}"}}}}}
python main.py check --config integration_tests/sample_config.json
{"type": "LOG", "log": {"level": "INFO", "message": "Check succeeded"}}
{"type": "CONNECTION_STATUS", "connectionStatus": {"status": "SUCCEEDED"}}
python main.py check --config integration_tests/invalid_config.json //using wrong Access key
{"type": "LOG", "log": {"level": "ERROR", "message": "Check failed"}}
{"type": "CONNECTION_STATUS", "connectionStatus": {"status": "FAILED", "message": "'Check Query and Access Key'"}}
python main.py discover --config integration_tests/sample_config.json
{"type": "CATALOG", "catalog": {"streams": [{"name": "current_weather", "json_schema": {"$schema": "http://json-schema.org/draft-07/schema#", "type": "object", "properties": {"request": {"type": ["null", "object"], "properties": {"type": {"type": "string"}, "query": {"type": "string"}, "language": {"type": "string"}, "unit": {"type": "string"}}}, "location": {"type":
["null", "object"], "properties": {"name": {"type": "string"}, "country":
{"type": "string"}, "region": {"type": "string"}, "lat": {"type": "string"}, "lon": {"type": "string"}, "timezone_id": {"type": "string"}, "localtime": {"type": "string"}, "localtime_epoch": {"type": "number"}, "utc_offset": {"type": "string"}}}, "current": {"type": ["null", "object"], "properties": {"observation_time": {"type": "string"}, "temperature": {"type": "number"}, "weather_code": {"type": "number"}, "weather_icons": {"type": "array"}, "weather_descriptions": {"type": "array"}, "wind_speed": {"type": "number"}, "wind_degree": {"type": "number"}, "wind_dir": {"type": "string"}, "pressure": {"type": "number"}, "precip": {"type": "number"}, "humidity": {"type": "number"}, "cloudcover": {"type": "number"}, "feelslike": {"type": "number"}, "uv_index": {"type": "number"}, "visibility": {"type": "number"}}}}}, "supported_sync_modes": ["full_refresh"]}, {"name": "forecast",
"json_schema": {"$schema": "http://json-schema.org/draft-07/schema#", "type": "object", "properties": {"request": {"type": ["null", "object"], "properties": {"type": {"type": "string"}, "query": {"type": "string"}, "language": {"type": "string"}, "unit": {"type": "string"}}}, "location": {"type": ["null", "object"], "properties": {"name": {"type": "string"}, "country": {"type": "string"}, "region": {"type": "string"}, "lat": {"type": "string"}, "lon": {"type": "string"}, "timezone_id": {"type": "string"}, "localtime": {"type": "string"}, "localtime_epoch": {"type": "number"}, "utc_offset": {"type": "string"}}}, "current": {"type": ["null", "object"], "properties": {"observation_time": {"type": "string"}, "temperature": {"type": "number"}, "weather_code": {"type": "number"}, "weather_icons": {"type": "array"}, "weather_descriptions": {"type": "array"}, "wind_speed": {"type": "number"}, "wind_degree": {"type": "number"}, "wind_dir": {"type": "string"}, "pressure": {"type": "number"}, "precip": {"type": "number"}, "humidity": {"type": "number"}, "cloudcover": {"type": "number"}, "feelslike": {"type": "number"}, "uv_index": {"type": "number"}, "visibility": {"type": "number"}}}, "forecast": {"type": ["null", "object"]}}}, "supported_sync_modes": ["full_refresh"]}, {"name": "location_lookup", "json_schema": {"$schema": "http://json-schema.org/draft-07/schema#", "type": "object", "properties": {"request": {"type": ["null", "object"], "properties": {"query": {"type": "string"}, "results": {"type": "number"}}}, "results": {"type": ["null", "array"], "properties": {"name": {"type": "string"}, "country": {"type": "string"}, "region": {"type": "string"}, "lon": {"type": "string"}, "lat": {"type": "string"}, "timezone_id": {"type": "string"}, "utc_offset": {"type": "string"}}}}}, "supported_sync_modes": ["full_refresh"]}, {"name": "historical", "json_schema": {"$schema": "http://json-schema.org/draft-07/schema#", "type": "object", "properties": {"request": {"type": ["null", "object"], "properties": {"type": {"type": "string"}, "query": {"type": "string"}, "language": {"type": "string"}, "unit": {"type": "string"}}}, "location": {"type": ["null", "object"], "properties": {"name": {"type": "string"}, "country": {"type": "string"}, "region": {"type": "string"}, "lat":
{"type": "string"}, "lon": {"type": "string"}, "timezone_id": {"type": "string"}, "localtime": {"type": "string"}, "localtime_epoch": {"type": "number"}, "utc_offset": {"type": "string"}}}, "current": {"type": ["null", "obe": {"type": "number"}, "weather_code": {"type": "number"}, "weather_icons": {"type": "array"}, "weather_descriptions": {"type": "array"}, "wind_speed": {"type": "number"}, "wind_degree": {"type": "number"}, "wind_dir": {"type": "string"}, "pressure": {"type": "number"}, "precip": {"type": "number"}, "humidity": {"type": "number"}, "cloudcover": {"type": "number"}, "feelslike": {"type": "number"}, "uv_index": {"type": "number"}, "visibility": {"type": "number"}}}, "historical": {"type": ["null", "object"]}}}, "supported_sync_modes": ["full_refresh"]}]}}
python main.py read --config integration_tests/sample_config.json --catalog integration_tests/catalog.json
{"type": "LOG", "log": {"level": "INFO", "message": "Starting syncing SourceWeatherstack"}}
{"type": "LOG", "log": {"level": "INFO", "message": "Syncing stream: current_weather "}}
{"type": "RECORD", "record": {"stream": "current_weather", "data": {"request": {"type": "City", "query": "Hong Kong, Hong Kong", "language": "en", "unit": "m"}, "location": {"name": "Hong Kong", "country": "Hong Kong", "region": "", "lat": "22.283", "lon": "114.150", "timezone_id": "Asia/Shanghai", "localtime": "2022-09-22 02:31", "localtime_epoch": 1663813860, "utc_offset": "8.0"}, "current": {"observation_time": "06:31 PM", "temperature": 28, "weather_code": 116, "weather_icons": ["https://assets.weatherstack.com/images/wsymbols01_png_64/wsymbol_0004_black_low_cloud.png"], "weather_descriptions": ["Partly cloudy"], "wind_speed": 9, "wind_degree": 120, "wind_dir": "ESE", "pressure":
1011, "precip": 0, "humidity": 70, "cloudcover": 50, "feelslike": 31, "uv_index": 1, "visibility":
10, "is_day": "no"}}, "emitted_at": 1663785109265}}
{"type": "LOG", "log": {"level": "INFO", "message": "Read 1 records from current_weather stream"}}
{"type": "LOG", "log": {"level": "INFO", "message": "Finished syncing current_weather"}}
{"type": "LOG", "log": {"level": "INFO", "message": "SourceWeatherstack runtimes:\nSyncing stream current_weather 0:00:00.400414"}}
{"type": "LOG", "log": {"level": "INFO", "message": "Syncing stream: forecast "}}
{"type": "RECORD", "record": {"stream": "forecast", "data": {"request": {"type": "City", "query": "Hong Kong, Hong Kong", "language": "en", "unit": "m"}, "location": {"name": "Hong Kong", "country": "Hong Kong", "region": "", "lat": "22.283", "lon": "114.150", "timezone_id": "Asia/Shanghai", "localtime": "2022-09-22 02:31", "localtime_epoch": 1663813860, "utc_offset": "8.0"}, "current": {"observation_time": "06:31 PM", "temperature": 28, "weather_code": 116, "weather_icons": ["https://assets.weatherstack.com/images/wsymbols01_png_64/wsymbol_0004_black_low_cloud.png"], "weather_descriptions": ["Partly cloudy"], "wind_speed": 9, "wind_degree": 120, "wind_dir": "ESE", "pressure": 1011, "precip": 0, "humidity": 70, "cloudcover": 50, "feelslike": 31, "uv_index": 1, "visibility": 10, "is_day": "no"}, "forecast": {"2022-09-21": {"date": "2022-09-21", "date_epoch": 1663718400, "astro":
{"sunrise": "06:11 AM", "sunset": "06:21 PM", "moonrise": "01:48 AM", "moonset": "03:41 PM", "moon_phase": "Waning Crescent", "moon_illumination": 15}, "mintemp": 27, "maxtemp": 28, "avgtemp": 27, "totalsnow": 0, "sunhour": 11.8, "uv_index": 6}}}, "emitted_at": 1663785109618}}
{"type": "LOG", "log": {"level": "INFO", "message": "Read 1 records from forecast stream"}}
{"type": "LOG", "log": {"level": "INFO", "message": "Finished syncing forecast"}}
{"type": "LOG", "log": {"level": "INFO", "message": "SourceWeatherstack runtimes:\nSyncing stream current_weather 0:00:00.400414\nSyncing stream forecast 0:00:00.357215"}}
{"type": "LOG", "log": {"level": "INFO", "message": "Syncing stream: historical "}}
{"type": "RECORD", "record": {"stream": "historical", "data": {"success": false, "error": {"code":
603, "type": "historical_queries_not_supported_on_plan", "info": "Your current subscription plan does not support historical weather data. Please upgrade your account to use this feature."}}, "emitted_at": 1663785109728}}
{"type": "LOG", "log": {"level": "INFO", "message": "Read 1 records from historical stream"}}
{"type": "LOG", "log": {"level": "INFO", "message": "Finished syncing historical"}}
{"type": "LOG", "log": {"level": "INFO", "message": "SourceWeatherstack runtimes:\nSyncing stream current_weather 0:00:00.400414\nSyncing stream forecast 0:00:00.357215\nSyncing stream historical 0:00:00.100389"}}
{"type": "LOG", "log": {"level": "INFO", "message": "Syncing stream: location_lookup "}}
{"type": "RECORD", "record": {"stream": "location_lookup", "data": {"success": false, "error": {"code": 105, "type": "function_access_restricted", "info": "Access Restricted - Your current Subscription Plan does not support this API Function."}}, "emitted_at": 1663785109839}}
{"type": "LOG", "log": {"level": "INFO", "message": "Read 1 records from location_lookup stream"}}
{"type": "LOG", "log": {"level": "INFO", "message": "Finished syncing location_lookup"}}
{"type": "LOG", "log": {"level": "INFO", "message": "SourceWeatherstack runtimes:\nSyncing stream current_weather 0:00:00.400414\nSyncing stream forecast 0:00:00.357215\nSyncing stream historical 0:00:00.100389\nSyncing stream location_lookup 0:00:00.109713"}}
{"type": "LOG", "log": {"level": "INFO", "message": "Finished syncing SourceWeatherstack"}}
Acceptance
Put your acceptance tests output here.