Skip to content

Conversation

@fivetran-surabhisingh
Copy link
Collaborator

Jira ticket

Closes <ADD TICKET LINK HERE, EACH PR MUST BE LINKED TO A JIRA TICKET>

Description of Change

<MENTION A SHORT DESCRIPTION OF YOUR CHANGES HERE>

Testing

<MENTION ABOUT YOUR TESTING DETAILS HERE, ATTACH SCREENSHOTS IF NEEDED (WITHOUT PII)>

Checklist

Some tips and links to help validate your PR:

  • Tested the connector with fivetran debug command.
  • Added/Updated example specific README.md file, refer here for template.
  • Followed Python Coding Standards, refer here

@fivetran-surabhisingh fivetran-surabhisingh self-assigned this Oct 31, 2025
@fivetran-surabhisingh fivetran-surabhisingh requested a review from a team as a code owner October 31, 2025 18:25
@fivetran-surabhisingh fivetran-surabhisingh added the hackathon For all the PRs related to the internal Fivetran 2025 Connector SDK Hackathon. label Oct 31, 2025
@github-actions github-actions bot added the size/S PR size: small label Oct 31, 2025
@github-actions
Copy link

🧹 Python Code Quality Check

⚠️ Flake8 has detected issues, please fix the issues before merging:

📎 Download full report from workflow artifacts.

📌 Only Python files changed in this PR were checked.

🔍 See how this check works

This comment is auto-updated with every commit.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds a new Gusto connector to fetch HR and payroll data from the Gusto API. However, the implementation uses an incorrect API pattern that is incompatible with the Fivetran Connector SDK.

Key Changes

  • Adds a new Gusto connector implementation at connectors/gusto/connector.py
  • Attempts to use decorator-based pattern with @connector decorator
  • Fetches employee data from Gusto API

Comment on lines +6 to +7
import requests
from fivetran_connector_sdk import connector, config, state, records, log, schema
Copy link

Copilot AI Oct 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BLOCKER: Incorrect SDK imports. The Fivetran Connector SDK does not export connector, config, state, records, or schema modules in this way. The correct imports should be: from fivetran_connector_sdk import Connector, from fivetran_connector_sdk import Logging as log, and from fivetran_connector_sdk import Operations as op. See template_example_connector/connector.py for the correct pattern.

Copilot generated this review using guidance from repository custom instructions.
Fetches HR and payroll data from Gusto API.
"""

import requests
Copy link

Copilot AI Oct 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing comment explaining the import. Every import must have an inline comment. Note that requests is provided by the SDK runtime and should not be included in requirements.txt. Add comment like: # For making HTTP API requests to Gusto (provided by SDK runtime)

Copilot generated this review using guidance from repository custom instructions.
Comment on lines +9 to +31
CONFIG = config.Config(
base_url=config.StringField(default="https://api.gusto.com"),
access_token=config.SecretField(description="OAuth2 access token")
)

SCHEMA = schema.Schema(
name="gusto_employees",
columns={
"id": schema.StringColumn(),
"first_name": schema.StringColumn(),
"last_name": schema.StringColumn(),
"email": schema.StringColumn(),
"status": schema.StringColumn(),
}
)

@connector(
name="GustoConnector",
version="0.1.0",
config=CONFIG,
schema=SCHEMA,
)
def run_connector(ctx: state.Context):
Copy link

Copilot AI Oct 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BLOCKER: The entire structure is incompatible with Fivetran Connector SDK. The SDK does not use decorators, Config classes, or Schema classes. Instead, it requires: (1) A schema(configuration: dict) function that returns a list of table dictionaries, (2) An update(configuration: dict, state: dict) function for data fetching, (3) A Connector object initialized with these functions: connector = Connector(update=update, schema=schema). Configuration should be provided via configuration.json file. See template_example_connector/connector.py for the correct implementation pattern.

Copilot generated this review using guidance from repository custom instructions.
Comment on lines +33 to +37
response = requests.get(f"{ctx.config.base_url}/v1/employees", headers=headers)
response.raise_for_status()

for emp in response.json():
records.write("gusto_employees", emp)
Copy link

Copilot AI Oct 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BLOCKER: Incorrect operations usage. The SDK does not have a records.write() function. Instead, use op.upsert(table='gusto_employees', data=emp) after importing from fivetran_connector_sdk import Operations as op. Each upsert call must be preceded by a comment explaining its purpose.

Copilot generated this review using guidance from repository custom instructions.
)
def run_connector(ctx: state.Context):
headers = {"Authorization": f"Bearer {ctx.config.access_token}"}
response = requests.get(f"{ctx.config.base_url}/v1/employees", headers=headers)
Copy link

Copilot AI Oct 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing retry logic for API request. Network calls must implement retry logic with exponential backoff for transient failures (network errors, rate limits, 5xx errors). Limit to 3-5 retry attempts and log each retry. Example: for attempt in range(__MAX_RETRIES): try: response = requests.get(...); break except (requests.Timeout, requests.ConnectionError) as e: if attempt == __MAX_RETRIES - 1: raise; sleep_time = min(60, 2 ** attempt); time.sleep(sleep_time)

Copilot generated this review using guidance from repository custom instructions.
Comment on lines +33 to +36
response = requests.get(f"{ctx.config.base_url}/v1/employees", headers=headers)
response.raise_for_status()

for emp in response.json():
Copy link

Copilot AI Oct 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potential memory issue: response.json() loads the entire response into memory. For large employee datasets, this could cause memory overflow. If the Gusto API supports pagination, implement it to process data in chunks. If pagination is not available and datasets are guaranteed to be small, add a comment explaining why full materialization is acceptable.

Copilot generated this review using guidance from repository custom instructions.
for emp in response.json():
records.write("gusto_employees", emp)

return ctx.update_state({"last_sync": "now"})
Copy link

Copilot AI Oct 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BLOCKER: Incorrect state management. The SDK does not use ctx.update_state(). Instead, state should be updated via op.checkpoint(state) where state is a dictionary. The update function should not return anything. State checkpointing must occur after successful data operations with the required comment: # Save the progress by checkpointing the state...

Copilot generated this review using guidance from repository custom instructions.
Comment on lines +1 to +4
"""
Gusto connector for Fivetran Connector SDK.
Fetches HR and payroll data from Gusto API.
"""
Copy link

Copilot AI Oct 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Module docstring should include a link to technical reference documentation. Expected format: Line 1 should be a one-line description, followed by reference links. Example from template: \"\"\"ADD ONE LINE DESCRIPTION OF YOUR CONNECTOR HERE.\\nSee the Technical Reference documentation (https://fivetran.com/docs/connectors/connector-sdk/technical-reference#update)\\nand the Best Practices documentation (https://fivetran.com/docs/connectors/connector-sdk/best-practices) for details\"\"\"

Copilot generated this review using guidance from repository custom instructions.
Comment on lines +1 to +9
"""
Gusto connector for Fivetran Connector SDK.
Fetches HR and payroll data from Gusto API.
"""

import requests
from fivetran_connector_sdk import connector, config, state, records, log, schema

CONFIG = config.Config(
Copy link

Copilot AI Oct 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing required structure. The connector is missing: (1) import json for configuration file reading, (2) schema(configuration: dict) function with required docstring, (3) update(configuration: dict, state: dict) function with required docstring, (4) Connector initialization: connector = Connector(update=update, schema=schema), (5) Main block for local debugging with if __name__ == '__main__': connector.debug(). See template_example_connector/connector.py lines 7-166 for the complete required structure.

Copilot generated this review using guidance from repository custom instructions.
"""

import requests
from fivetran_connector_sdk import connector, config, state, records, log, schema
Copy link

Copilot AI Oct 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Import of 'log' is not used.

Suggested change
from fivetran_connector_sdk import connector, config, state, records, log, schema
from fivetran_connector_sdk import connector, config, state, records, schema

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

hackathon For all the PRs related to the internal Fivetran 2025 Connector SDK Hackathon. size/S PR size: small

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant