Skip to content

Conversation

@fivetran-adamrees
Copy link
Collaborator

Description of Change

Adding a new connection to the example repository

Testing

This is actively being used by a customer. Also, tested locally this morning here
image

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-adamrees fivetran-adamrees requested a review from a team as a code owner October 27, 2025 19:51
@fivetran-adamrees fivetran-adamrees added the hackathon For all the PRs related to the internal Fivetran 2025 Connector SDK Hackathon. label Oct 27, 2025
@fivetran-adamrees fivetran-adamrees requested a review from a team as a code owner October 27, 2025 19:51
@github-actions github-actions bot added the size/L PR size: Large label Oct 27, 2025
@github-actions
Copy link

github-actions bot commented Oct 27, 2025

🧹 Python Code Quality Check

✅ No issues found in Python Files.

🔍 See how this check works

This comment is auto-updated with every commit.

@fivetran-adamrees fivetran-adamrees changed the title Adding Connector SDK example for PeopleAI example(PeopleAI): add people_ai connector example Oct 27, 2025
@fivetran-sahilkhirwal fivetran-sahilkhirwal requested review from Copilot, fivetran-pranavtotala and fivetran-sahilkhirwal and removed request for a team October 28, 2025 14:56
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 People.ai connector example that demonstrates how to sync activity and participant data from the People.ai API using OAuth2 authentication with client credentials. The connector implements token refresh logic, exponential backoff retry handling, and offset-based pagination.

Key Changes

  • New People.ai connector with OAuth2 authentication and automatic token refresh on 401 errors
  • Implements retry logic with exponential backoff for transient 5xx and connection errors
  • Syncs two tables: activity (base activities) and participants (participant details)

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 29 comments.

File Description
connectors/people_ai/connector.py Core connector implementation with authentication, pagination, error handling, and data sync logic
connectors/people_ai/configuration.json Configuration file with API key and secret placeholders
connectors/people_ai/README.md Documentation covering connector overview, features, authentication, pagination, data handling, and error handling
README.md Added People.ai connector to the main repository connector list

Copy link
Contributor

@5tran-alexil 5tran-alexil left a comment

Choose a reason for hiding this comment

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

See my suggestions. Otherwise, README looks good.

@mesmith027
Copy link
Collaborator

Looks good; this is ready for an engineering review! 🥳 🎉

Copy link
Contributor

@5tran-alexil 5tran-alexil left a comment

Choose a reason for hiding this comment

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

See my suggestions

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

Copilot reviewed 5 out of 5 changed files in this pull request and generated 15 comments.

Comment on lines +374 to +386
def update(configuration: dict, state: dict):
"""
Define the update function which lets you configure
how your connector fetches data.
See the technical reference documentation for more details
on the update function: https://fivetran.com/docs/connectors/connector-sdk/technical-reference#update
Args:
configuration: A dictionary containing connection details.
state: A dictionary containing state information from previous runs.
The state dictionary is empty for the first sync or for any
full re-sync.
"""
Copy link

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

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

The update() function is missing a required validate_configuration() function call. According to SDK guidelines, every connector must implement and call validate_configuration() at the start of the update() function to validate configuration parameters (api_key and api_secret in this case).

Add this function before update():

def validate_configuration(configuration: dict):
    """
    Validate the configuration dictionary to ensure it contains all required parameters.
    This function is called at the start of the update method to ensure that the connector has all necessary configuration values.
    Args:
        configuration: a dictionary that holds the configuration settings for the connector.
    Raises:
        ValueError: if any required configuration parameter is missing.
    """
    required_configs = ["api_key", "api_secret"]
    for key in required_configs:
        if key not in configuration:
            raise ValueError(f"Missing required configuration value: {key}")

Then call it at the start of update():

def update(configuration: dict, state: dict):
    """..."""
    validate_configuration(configuration)
    # rest of the code

Copilot generated this review using guidance from repository custom instructions.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

made this change. Earlier review said to not have a validate_configuration function

Copy link
Contributor

@fivetran-pranavtotala fivetran-pranavtotala left a comment

Choose a reason for hiding this comment

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

LGTM

@fivetran fivetran deleted a comment from cla-assistant bot Jan 2, 2026
Copy link
Contributor

@5tran-alexil 5tran-alexil left a comment

Choose a reason for hiding this comment

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

@fivetran-adamrees @fivetran-pranavtotala READMEs LGTM, i fixed a merge conflict

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/L PR size: Large

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants