-
Notifications
You must be signed in to change notification settings - Fork 2
add error handling example #115
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: main
Are you sure you want to change the base?
Conversation
|
…o fivetran-elijahdavis-patch-1
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.
please add PR description and follow existing examples, to ensure this one looks of similar quality and comments.
from fivetran_connector_sdk import Connector | ||
from fivetran_connector_sdk import Operations as op | ||
from fivetran_connector_sdk import Logging as log | ||
import time | ||
import json | ||
import sys | ||
import random |
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.
please add the comments for packages, refer our existing examples.
# Initialize logging level | ||
log.LOG_LEVEL = log.Level.INFO |
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.
following this pattern customers can push all the SEVERE logs as INFO, we should make the necessary changes as customers can directly copy the examples and modify them.
Error simulation types: | ||
0: No error simulation (normal operation) | ||
1: Missing Configuration error | ||
2: Invalid State Format error | ||
3: Data Generation Error | ||
4: Processing Error | ||
5: Database Operation Error |
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 should match with sections or methods so that its easy to follow along.
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.
See updated logic above ^^ https://github.com/fivetran/fivetran_connector_sdk/pull/115/files#r2047924848
{"table": "demo_response_test", "primary_key": ["key"]} | ||
] | ||
|
||
def update(configuration: dict, state: dict): |
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.
the update method is very long, please split it into small method where each method simulates an error type
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.
Something like this? It cuts the update code length down ~50% from 130+ lines down to 60.
Part 1:
def simulate_missing_configuration_error():
"""
Simulate error type 1: Missing Configuration error.
This method demonstrates how to handle missing required configuration parameters.
"""
log.info("Simulating missing configuration error")
validate_configuration({}) # Pass empty config to trigger error
def simulate_invalid_state_error():
"""
Simulate error type 2: Invalid State Format error.
This method demonstrates how to handle invalid state format.
"""
log.info("Simulating invalid state format error")
validate_state("invalid_state") # Pass invalid state to trigger error
def simulate_data_generation_error():
"""
Simulate error type 3: Data Generation Error.
This method demonstrates how to handle errors during data generation.
"""
log.info("Simulating data generation error")
handle_critical_error("Data generation error", "Simulated error during data generation")
def simulate_processing_error():
"""
Simulate error type 4: Processing Error.
This method demonstrates how to handle errors during data processing.
"""
log.info("Simulating data processing error")
handle_critical_error("Data processing error", "Simulated error during data processing")
def simulate_database_operation_error():
"""
Simulate error type 5: Database Operation Error.
This method demonstrates how to handle errors during database operations.
"""
log.info("Simulating database operation error")
handle_critical_error("Database operation error", "Simulated error during database operations")
Part 2.
def update(configuration: dict, state: dict):
"""
Main update function that simulates data processing with various error scenarios.
Error simulation types:
0: No error simulation (normal operation)
1: Missing Configuration error
2: Invalid State Format error
3: Data Generation Error
4: Processing Error
5: Database Operation Error
Args:
configuration: Dictionary containing configuration parameters
state: Dictionary containing the current state/cursor information
Yields:
Operations for upserting data and checkpointing state
"""
log.info("Starting update process")
# Get error simulation type from configuration
error_simulation_type = str(configuration.get('error_simulation_type', "0"))
log.info(f"Error simulation type: {error_simulation_type}")
if error_simulation_type == "0":
log.info("Running in normal mode - no error simulation")
# Section 1: Configuration Validation
if error_simulation_type == "1":
simulate_missing_configuration_error()
else:
validate_configuration(configuration)
# Section 2: State Validation
if error_simulation_type == "2":
simulate_invalid_state_error()
else:
validate_state(state)
# Section 3: Data Generation
if error_simulation_type == "3":
simulate_data_generation_error()
response = generate_data(configuration)
# Section 4: Data Processing
if error_simulation_type == "4":
simulate_processing_error()
processed_data, new_state = process_data(response, configuration)
# Section 5: Database Operations
record = {
"key": datetime.now(timezone.utc).timestamp(),
"request": configuration['prompt'],
"result": processed_data,
"state": json.dumps(new_state)
}
if error_simulation_type == "5":
simulate_database_operation_error()
yield from perform_database_operations(record)
Create connector instance
connector = Connector(update=update, schema=schema)
Co-authored-by: Varun Dhall <[email protected]>
Co-authored-by: Varun Dhall <[email protected]>
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.
Love the idea behind this one :-)
if error_simulation_type == "1": | ||
log.info("Simulating missing configuration error") | ||
log.info("BEST PRACTICE: Always validate required configuration parameters early") | ||
raise KeyError("demo_config missing from configuration") |
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.
should we standardize on how we recommend this is handled?
I found myself adding similar code here:
https://github.com/fivetran/fivetran_connector_sdk/blob/main/examples/common_patterns_for_connectors/cursors/marketstack/connector.py#L83
What I don't like about this example is you aren't actually validating the configuration object.
you are trigging straight from the error_simulation ENUM
hum maybe I should have thrown a KeyError rather than a ValueError....
log.info("Starting update process") | ||
|
||
# Get error simulation type from configuration | ||
error_simulation_type = str(configuration.get('error_simulation_type', "0")) |
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.
Shouldn't we be following our own suggested best practice here and check configuration for critical keys eg 'error_simulation_type' and throw a human readable error if the connector builder has run this example forgetting to pass in --configuration myfile.json?
if error_simulation_type == "2": | ||
log.info("Simulating invalid state format error") | ||
log.info("BEST PRACTICE: Validate state format before using it") | ||
handle_critical_error("Invalid state format detected") |
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.
what would an example of this actually be?
where you check the state for type?
what sort of formats are often required
this could also be where you might cast your string (what we store in state) into a timestamp.
Main update function that simulates data processing with various error scenarios. | ||
|
||
Error simulation types: | ||
0: No error simulation (normal operation) |
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.
does this example have to run correctly the first time before you start to play with the various simulated failure states?
would this be a pattern better suited to the playground?
This is the tutorial/enablement-focused connector that was written for Andersen. The customer can use 0-5 in the configuration.json "error_simulation_type" key to see multiple scenarios that are listed below.
Rough outline: