Skip to content

Conversation

@Powlinett
Copy link
Member

Proposed changes

  • automated code changes:
    • add settings.py
    • update connector.py
    • update main.py or __main__.py
    • add unit tests

Related issues

Checklist

  • I consider the submitted work as finished
  • I have signed my commits using GPG key.
  • I tested the code for its functionality using different use cases
  • I added/update the relevant documentation (either on github or on notion)
  • Where necessary I refactored code to improve the overall quality

Further comments

The code needs to be reviewed by two people: one must fix any issue, the other one review the final commits.

@Powlinett Powlinett added filigran team use to identify PR from the Filigran team do not merge Do not merge this PR until this tag will be removed composer connector: domaintools labels Dec 4, 2025
@Powlinett Powlinett linked an issue Dec 4, 2025 that may be closed by this pull request
@Powlinett Powlinett force-pushed the feat/4847-migrate-domaintools branch from 3c787ac to 68a0c0a Compare December 9, 2025 11:54
@mariot mariot self-assigned this Dec 23, 2025
@mariot mariot force-pushed the feat/4847-migrate-domaintools branch 2 times, most recently from 5bf48c0 to 6f3d4e2 Compare December 23, 2025 18:25
@mariot mariot removed the do not merge Do not merge this PR until this tag will be removed label Dec 23, 2025
Copy link
Member

@jabesq jabesq left a comment

Choose a reason for hiding this comment

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

See comments, I will push the changes to fix it

)
scope: ListFromString = Field(
description="The scope of the connector.",
default=["Domain-Name,Ipv4-Addr"],
Copy link
Member

Choose a reason for hiding this comment

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

issue: It should be changed as follow, otherwise it will set an invalid scope

Suggested change
default=["Domain-Name,Ipv4-Addr"],
default=["Domain-Name", "Ipv4-Addr"],

jabesq
jabesq previously approved these changes Dec 30, 2025
@jabesq jabesq force-pushed the feat/4847-migrate-domaintools branch from 5f4bcb4 to bf7035c Compare January 5, 2026 13:58
@SamuelHassine SamuelHassine requested a review from Copilot January 8, 2026 21:05
Copy link

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 updates the domaintools connector to support the "manager_supported" pattern by integrating with the connectors-sdk framework. The changes introduce a settings module using Pydantic for configuration validation, refactor the connector initialization to use dependency injection, and add comprehensive unit tests.

Key changes:

  • Introduces settings.py with Pydantic-based configuration models extending BaseConnectorSettings
  • Refactors connector initialization to accept config and helper as constructor parameters
  • Adds comprehensive unit tests for settings validation and connector instantiation

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/connector/settings.py Defines configuration models using Pydantic and connectors-sdk base classes
src/connector/connector.py Refactors constructor to accept config/helper parameters and removes manual config parsing
src/connector/builder.py Updates to use OrganizationAuthor model from connectors-sdk
src/main.py Implements new entry point with settings instantiation and error handling
src/requirements.txt Adds pydantic and connectors-sdk dependencies
tests/tests_connector/test_settings.py Adds unit tests for settings validation
tests/test_main.py Adds integration tests for connector instantiation
tests/conftest.py Configures pytest to include src directory in path
tests/test-requirements.txt Specifies pytest as test dependency
src/config.yml.sample Updates configuration template with commented defaults
docker-compose.yml Updates environment variables, comments optional settings
metadata/connector_config_schema.json Adds JSON schema for connector configuration
metadata/CONNECTOR_CONFIG_DOC.md Adds documentation for configuration parameters

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

validators~=0.35.0
validators~=0.35.0
pydantic >=2.8.2, <3
connectors-sdk @ git+https://github.com/OpenCTI-Platform/connectors.git@master#subdirectory=connectors-sdk
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

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

Installing from a git branch (master) can lead to unpredictable builds as the dependency may change without notice. Consider pinning to a specific commit hash or tagged release version for reproducible builds.

Suggested change
connectors-sdk @ git+https://github.com/OpenCTI-Platform/connectors.git@master#subdirectory=connectors-sdk
connectors-sdk @ git+https://github.com/OpenCTI-Platform/connectors.git@6.9.5#subdirectory=connectors-sdk

Copilot uses AI. Check for mistakes.
class FakeConnectorSettings(ConnectorSettings):
"""
Subclass of `ConnectorSettings` (implementation of `BaseConnectorSettings`) for testing purpose.
It overrides `BaseConnectorSettings._load_config_dict` to return a fake but valid config dict.
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

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

The docstring states 'fake but valid config dict' but the test function is specifically testing invalid inputs. The docstring should be corrected to say 'fake and invalid config dict' to accurately reflect the test's purpose.

Suggested change
It overrides `BaseConnectorSettings._load_config_dict` to return a fake but valid config dict.
It overrides `BaseConnectorSettings._load_config_dict` to return a fake and invalid config dict.

Copilot uses AI. Check for mistakes.
if expiration_date != "":
expiration_date = datetime.strptime(expiration_date, "%Y-%m-%d")

if creation_date >= expiration_date:
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

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

The comparison should check if creation_date is strictly after expiration_date (>) rather than greater-or-equal (>=). If both dates are equal, they represent the same point in time and should be valid, not requiring the warning and date removal.

Suggested change
if creation_date >= expiration_date:
if creation_date > expiration_date:

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

composer connector: domaintools filigran team use to identify PR from the Filigran team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[domaintools] Migrate connector to be connector manager supported

4 participants