Skip to content

feat(source-float): add reports-people and reports-projects streams - #79087

Open
Pradip (masterpradipg) wants to merge 10 commits into
airbytehq:masterfrom
masterpradipg:masterpradipg/source-float-add-reports-streams
Open

feat(source-float): add reports-people and reports-projects streams#79087
Pradip (masterpradipg) wants to merge 10 commits into
airbytehq:masterfrom
masterpradipg:masterpradipg/source-float-add-reports-streams

Conversation

@masterpradipg

Copy link
Copy Markdown

What

Adds two new streams to the source-float connector using the Float Reports API:

  • reports-people — aggregated scheduling data per person for a given date range
  • reports-projects — aggregated scheduling data per project for a given date range

Also adds an optional end_date config field (YYYY-MM-DD) to bound the report date range.

How

  • Added reports-people stream: GET /reports/people with start_date and end_date query params, response unwrapped from people root key via DpathExtractor
  • Added reports-projects stream: GET /reports/projects with start_date and end_date query params, response unwrapped from projects root key via DpathExtractor
  • Both streams use NoPagination — the Float API explicitly states pagination is not available on report endpoints
  • start_date is derived from the existing config field (trimmed to YYYY-MM-DD)
  • end_date is optional and defaults to today via now_utc() if not provided
  • Bumped connector version 0.0.470.1.0 (minor bump per Airbyte semver: adding streams)

Review guide

  1. airbyte-integrations/connectors/source-float/manifest.yaml — two new stream definitions after logged-time (lines ~629–684), two new schemas at the bottom, end_date added to spec
  2. airbyte-integrations/connectors/source-float/metadata.yaml — version bump only
  3. docs/integrations/sources/float.mdend_date added to config table, two new rows in streams table, changelog entry for 0.1.0

The existing 16 stream definitions and their schemas are untouched.

User Impact

Users can now sync aggregated utilization and scheduling report data from Float — hours per person (capacity, billable, overtime, etc.) and hours per project — directly into their
destination without manually calling the Float Reports API.

No impact on existing streams or configurations.

Can this PR be safely reverted and rolled back?

  • YES 💚

@CLAassistant

CLAassistant commented Jun 2, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@octavia-bot
octavia-bot Bot marked this pull request as draft June 2, 2026 09:58
@octavia-bot

octavia-bot Bot commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Note

📝 PR Converted to Draft

More info...

Thank you for creating this PR. As a policy to protect our engineers' time, Airbyte requires all PRs to be created first in draft status. Your PR has been automatically converted to draft status in respect for this policy.

As soon as your PR is ready for formal review, you can proceed to convert the PR to "ready for review" status by clicking the "Ready for review" button at the bottom of the PR page.

To skip draft status in future PRs, please include [ready] in your PR title or add the skip-draft-status label when creating your PR.

@github-actions

github-actions Bot commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

👋 Welcome to Airbyte!

Thank you for your contribution from masterpradipg/airbyte! We're excited to have you in the Airbyte community.

If you have any questions, feel free to ask in the PR comments or join our Slack community.

💡 Show Tips and Tricks

PR Slash Commands

As needed or by request, Airbyte Maintainers can execute the following slash commands on your PR:

  • /format-fix - Fixes most formatting issues.
  • /bump-version - Bumps connector versions.
  • /run-connector-tests - Runs connector tests.
  • /run-cat-tests - Runs CAT tests.
  • /run-regression-tests - Runs regression tests for the modified connector(s).
  • /build-connector-images - Builds and publishes a pre-release docker image for the modified connector(s).
  • /publish-connectors-prerelease - Publishes pre-release connector builds (tagged as {version}-preview.{git-sha}) for all modified connectors in the PR.
  • /ai-review - AI-powered PR review for connector safety and quality gates.
  • /ai-docs-review - AI-powered documentation review for PRs with connector changes.
  • /ai-create-docs-pr - Creates a documentation PR for connector changes.
  • /force-merge reason="<A_GOOD_REASON>" - Force merges the PR using admin privileges, bypassing CI checks. Requires a reason.

Tips for Working with CI

  1. Pre-Release Checks. Please pay attention to these, as they contain standard checks on the metadata.yaml file, docs requirements, etc. If you need help resolving a pre-release check, please ask a maintainer.
    • Note: If you are creating a new connector, please be sure to replace the default logo.svg file with a suitable icon.
  2. Connector CI Tests. Some failures here may be expected if your tests require credentials. Please review these results to ensure (1) unit tests are passing, if applicable, and (2) integration tests pass to the degree possible and expected.
  3. (Optional.) BYO Connector Credentials for tests in your fork. You can optionally set up your fork with BYO credentials for your connector. This can significantly speed up your review, ensuring your changes are fully tested before the maintainers begin their review.
📚 Show Repo Guidance

Helpful Resources

📝 Edit this welcome message.

@masterpradipg
Pradip (masterpradipg) force-pushed the masterpradipg/source-float-add-reports-streams branch from 249688c to 72327c7 Compare June 2, 2026 10:47
@masterpradipg Pradip (masterpradipg) changed the title ✨ Souprce Float: add reports-people and reports-projects streams feat(source-float): add reports-people and reports-projects streams Jun 2, 2026
@masterpradipg
Pradip (masterpradipg) marked this pull request as ready for review June 2, 2026 10:56
…eams

Resolve conflicts in source-float metadata.yaml and float.md against
upstream dependency bumps (0.0.47 -> 0.0.56):
- keep dockerImageTag 0.1.0 for the new reports streams
- keep the 0.1.0 changelog entry above the upstream dependency rows
@airbyte-support-bot

Copy link
Copy Markdown
Contributor

↪️ Triggering /ai-review per Hands-Free AI Triage Project triage next step.

Reason: PR is ready for review and has not had an AI review yet; review is the next pipeline step.

Devin session

@octavia-bot

octavia-bot Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

AI PR Review starting...

Reviewing PR for connector safety and quality.
View playbook

Devin AI session created successfully!

@airbyte-support-bot

Airbyte Support Bot (airbyte-support-bot) commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

🛡️ AI PR Review Report

🔴 Review Action: REQUEST CHANGES

Gate Status
Test Coverage FAIL
Code Hygiene WARNING
Live / E2E Tests UNKNOWN

🔶 Risk Level: 2/5

Additive-only change: two new report streams, one new optional end_date spec field, minor version bump — no existing stream, schema, or config behavior touched.

🔧 Remediation Required

Test Coverage (enforced, FAIL)
This PR adds new functionality (feat / "add" in the title, reports-people and reports-projects streams) but contains no test files. airbyte-integrations/connectors/source-float/ currently has no unit_tests/ or integration_tests/ directory at all.

Add at least one test that validates the new behavior, e.g. a mock-server / unit test under airbyte-integrations/connectors/source-float/unit_tests/ covering:

  • the people / projects root-key extraction (DpathExtractor),
  • the start_date trim (config['start_date'][:10]) and the end_date default (now_utc()) request parameters,
  • the record shape against the new inline schemas.

Live / E2E Tests (UNKNOWN)
No /ai-prove-fix run was found for this PR. Because the change adds streams (discovery/catalog + sync behavior), validation is required. Run /ai-prove-fix on this PR to publish a pre-release and validate the new streams against the Float API.

Code Hygiene (WARNING, non-blocking)
Same root cause as Test Coverage — manifest.yaml changed with no accompanying test files. No live validation evidence is present, so the coverage risk is not mitigated.


📋 PR Details

Connector(s): source-float
PR: #79087
HEAD SHA: 09f4a034ba385a2592943c64a82fa2472114c283
Session: https://app.devin.ai/sessions/e75e1b991d8f431ab3600ec4aa855bc7

🔍 Gate Evaluation Details

Gate-by-Gate Analysis

Gate Status Enforced? Details
PR Hygiene PASS Yes Description is filled in (What / How / Review guide / User Impact, ~1.9k chars); docs/integrations/sources/float.md changelog updated with the 0.1.0 row; no unresolved human review comments.
Code Hygiene WARNING WARNING manifest.yaml (source) modified; no test files modified, and the connector has no test directory.
Test Coverage FAIL Yes Behavioral-change indicators matched (title feat(...) / "add"); zero test files in the diff and none exist for this connector. Manifest-only changes are not exempt.
Code Security PASS Yes No changed file matches auth/credential/token path patterns. Diff hunks in manifest.yaml contain no authenticator/api_token/client_secret/*_token keywords (new streams $ref the existing base_requester); metadata.yaml hunk is dockerImageTag only.
Per-Record Performance PASS WARNING New per-record path is a plain DpathExtractor on a root key with no transformations, regex, or nested iteration.
Breaking Dependencies PASS WARNING No pyproject.toml, poetry.lock, or Gradle dependency changes in the diff.
Backwards Compatibility PASS Yes (enforced) Spec comparison performed against origin/master — no required array exists in either version (master: none; PR: none), so no required field was added. end_date is a new optional property. No stream removed/renamed, no primary key or cursor changed, no existing schema property or type/format changed — the two new inline schemas are entirely new. Minor bump 0.0.56 → 0.1.0 is appropriate for additive streams on a pre-1.0 connector.
Forwards Compatibility PASS Warning (elevates Risk Level) paginator: NoPagination / record_selector appear only inside the two brand-new stream definitions; no state, cursor, checkpoint, incremental_sync, partition_router, or transformations keyword appears anywhere in the diff hunks, and no existing stream's state shape changes. Rollback simply drops the new streams.
Behavioral Changes PASS Warning (elevates Risk Level) Keyword scan over diff hunks found no rate_limit, throttle, retry, backoff, Retry-After, timeout, sleep, error_handler, response_filters, http_codes, or page_size matches.
Out-of-Scope Changes PASS Skip All 3 changed files are in scope: 2 under airbyte-integrations/connectors/source-float/, 1 under docs/.
CI Checks PASS Yes All core checks green on the HEAD SHA, including Lint source-float Connector, Test source-float Connector [No Creds], Build and Verify Artifacts (source-float), Format Check, Check Changelog Updated, Docs / Vale, Docs / MarkDownLint. The two failures — source-float Progressive Rollout Gate and Connector Active Progressive Rollout Checks Summary — are release-management checks explicitly excluded from this gate (they reflect the connector's rollout state on the base branch, not a defect in this PR).
Live / E2E Tests UNKNOWN Yes Validation IS required (new streams ⇒ discovery/catalog change and sync-behavior change), but no /ai-prove-fix conclusion was found on the PR. Not run ⇒ UNKNOWN, not FAIL.

Spec Comparison (manifest.yamlspec.connection_specification):

  • Master required: (no required array declared)
  • PR required: (no required array declared)
  • New properties: end_date (optional, pattern ^[0-9]{4}-[0-9]{2}-[0-9]{2}$, defaults to today via now_utc()) → additive, non-breaking
📚 Evidence Consulted

Evidence

  • Changed files: 3 files (source-float/manifest.yaml +156/-0, source-float/metadata.yaml +1/-1, docs/integrations/sources/float.md +4/-0)
  • CI checks: 32 check-runs on 09f4a03 — 22 success, 8 skipped, 2 failure (source-float Progressive Rollout Gate, Connector Active Progressive Rollout Checks Summary — both excluded from CI Checks per playbook)
  • PR labels: community, connectors/source/float, hyd-review (no validation/override labels)
  • PR description: present (~1.9k chars, structured)
  • Existing bot reviews: none
  • /ai-prove-fix runs: none found
❓ How to Respond

Resolving a Failing Gate

Each non-PASS gate above lists concrete remediation. Apply it, push, then re-run /ai-review — the bot re-evaluates from the diff, CI check-runs, and /ai-prove-fix results.

Written explanations do not change a gate verdict. If you believe a gate is a false positive or it cannot be remediated as written, request review from a human maintainer and explain the situation to them in a PR comment — a human reviewer can approve or merge over a bot FAIL. Never edit the PR description to argue with the bot; the description is how reviewers learn what the PR does.

Pradip (masterpradipg) and others added 2 commits August 12, 2026 18:39
…jects

Addresses the AI PR review Test Coverage gate on airbytehq#79087: source-float had no
unit_tests project at all.

Covers the behavior the two new report streams introduce:
- root-key extraction (`people` / `projects`) via DpathExtractor, including
  sibling keys in the payload and null-valued fields
- `start_date` trimmed from the config date-time to `YYYY-MM-DD`, `end_date`
  taken from config and defaulting to today via `now_utc()`
- NoPagination: exactly one request, no `page` / `per-page` params
- bearer auth, primary keys, full-refresh-only, inline schema vs emitted record
- `end_date` declared optional in the spec, and the 16 pre-existing streams
  still discoverable

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@bazarnov
Baz (bazarnov) self-requested a review August 12, 2026 14:22
…nd harden the new tests

- Both report streams now carry start_date/end_date on every record via
  AddFields, using the same expressions as the request parameters - the
  aggregates are meaningless downstream without the period they cover, and
  the values change between syncs as the schedule changes.
- Docs: report streams return one aggregated snapshot row per person or
  project for the configured range; full refresh overwrite recommended;
  end_date must not precede start_date.
- Tests: the no-pagination test now fails fast instead of hanging if a
  paginator is ever added; 19 redundant per-field tests collapsed into a
  strict full-record equality round-trip; conftest owns the manifest/source
  helpers; unit_tests/__init__.py removed so conftest imports resolve
  (package-style unit_tests break pytest's conftest import path);
  wk_day_hrs fixture matches the documented shape.
@bazarnov

Copy link
Copy Markdown
Contributor

Pradip (@masterpradipg) thanks for a genuinely well-built contribution - the stream definitions match Float's spec exactly (we verified every path, parameter, root key and all 19 schema fields against the swagger source behind developer.float.com), the primary keys are valid (the reports return one aggregated row per person/project for the whole range), and the tests you added after the review bot's remediation are properly discriminating - we mutation-tested them and every wrong manifest value was caught.

We pushed one commit on top (0a7702a5) with a few things that go beyond your original scope, so please take a look and confirm you are fine with them:

  1. Records now carry the period they cover. Both report streams stamp start_date/end_date onto every record (same expressions as the request parameters) and the schemas declare the two fields. Without this, the aggregates are ambiguous downstream - "10 scheduled hours" could mean a week or three years, and the values change every sync as the schedule moves. The docs now recommend full refresh overwrite and explain the snapshot semantics.
  2. Test suite restructuring. The no-pagination test previously hung (>75s) instead of failing if a paginator were ever added - it now fails fast on a finite mock. The 19 per-field tests were collapsed into one strict full-record equality round-trip (same detection power - we re-verified the mutations still fail - at ~60% of the runtime). Manifest/source helpers moved into conftest.py, and unit_tests/__init__.py was removed: package-style unit_tests break pytest's from conftest import resolution, and source-float was the only connector combining the two.
  3. Small accuracy fixes: the wk_day_hrs fixture now uses Float's documented shape ({"1970-01-01": [0, 8, 8, 8, 8, 8, 0]} - date-keyed Sun-Sat array), and the docs note that end_date must not precede start_date (Float's behavior for inverted ranges is undocumented).

Two pre-existing gaps we noticed but deliberately left out of this PR, in case you want follow-ups: Float documents a required User-Agent header ("name of your application and a contact email") which the connector never sends, and base_requester has no error handler despite documented 429 rate limits (30 req/min on report endpoints). Both affect all 18 streams equally.

Everything is green on the pushed head - unit tests (33), lint, and the connector QA checks. If the three items above look right to you, we will approve.

@devin-ai-integration

Copy link
Copy Markdown
Contributor

↪️ Triggering /ai-review per Hands-Free AI Triage Project triage next step.

Reason: The most recent AI review marker (FAIL) was recorded for 09f4a034 and the author has since pushed new commits; HEAD is now 0a7702a5, so a refreshed AI review is the next step.

Devin session

@octavia-bot

octavia-bot Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

AI PR Review starting...

Reviewing PR for connector safety and quality.
View playbook

Devin AI session created successfully!

@airbyte-support-bot

Airbyte Support Bot (airbyte-support-bot) commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

🛡️ AI PR Review Report

🔴 Review Action: REQUEST CHANGES

Gate Status
PR Hygiene FAIL
Per-Record Performance WARNING
Live / E2E Tests UNKNOWN

🔶 Risk Level: 2/5

Additive-only change in a single connector: two new full-refresh report streams, one new optional spec field, and a minor version bump; no existing stream, schema, spec requirement, or state format is touched.

🔧 Remediation Required

PR Hygiene — unresolved human reviewer feedback

Baz (@bazarnov)'s review comment (2026-08-13) explicitly asks the author to confirm three changes pushed on top of the original scope (start_date/end_date stamping on records, test-suite restructuring, wk_day_hrs fixture + docs accuracy fixes) and closes with "If the three items above look right to you, we will approve." There is no reply from Pradip (@masterpradipg) and no resolved thread.

→ Reply to that comment confirming (or objecting to) the three items, then re-run /ai-review.

Live / E2E Tests — UNKNOWN (not blocking)

No /ai-prove-fix run exists for this PR. Adding two new streams matches the "stream discovery / catalog generation" category, so validation is expected. Run /ai-prove-fix on this PR for pre-release validation evidence.


📋 PR Details

Connector(s): source-float
PR: #79087
HEAD SHA: 0a7702a5e226ab825f2ded28dad9df06950074cc
Session: https://app.devin.ai/sessions/10b73161f40845fa8dcb8376dd50dd08

This is a re-run: the prior review (FAIL, 09f4a034) flagged missing test coverage, which has since been remediated by unit_tests/test_reports_streams.py (366 lines) plus conftest.py.

🔍 Gate Evaluation Details

Gate-by-Gate Analysis

Gate Status Enforced? Details
PR Hygiene FAIL Yes Description (2,089 chars) and docs/integrations/sources/float.md changelog entry for 0.1.0 are both present; unresolved human reviewer comment from Baz (@bazarnov) requesting author confirmation.
Code Hygiene PASS WARNING Source (manifest.yaml) modified and test files modified (unit_tests/test_reports_streams.py, unit_tests/conftest.py).
Test Coverage PASS Yes Feature PR (feat(...), title keyword add); new test content present — def test_* functions, pytest fixtures/parametrize, and assertions in unit_tests/test_reports_streams.py.
Code Security PASS Yes No changed file path matches auth/credential/token/secret patterns. Diff-hunk keyword scan matched client_id, but only as a Float data field (reports-projects schema property, integer client identifier) and in a test fixture — no authenticator, credential, or allowedHosts change.
Per-Record Performance WARNING WARNING Both new streams run an AddFields transformation per record whose end_date value calls now_utc().strftime('%Y-%m-%d'), so the Jinja expression is evaluated once per record. Report endpoints return one aggregated row per person/project, so record counts are small and the impact is negligible — noted for awareness only.
Breaking Dependencies PASS WARNING New unit_tests/pyproject.toml + unit_tests/poetry.lock are test-scoped only; no runtime/connector dependency version changed.
Backwards Compatibility PASS Yes (enforced) Spec comparison — master required: ["access_token", "start_date"]; PR required: ["access_token", "start_date"] (unchanged). end_date is added as an optional property with a default. No stream removed/renamed, no PK or cursor change, no type/format change on any existing field; the 16 existing streams and their schemas are untouched. Minor bump 0.0.560.1.0 is appropriate for an additive pre-1.0 change.
Forwards Compatibility PASS Warning (elevates Risk Level) transformations/record_selector/paginator keywords appear only inside the two new full-refresh stream definitions. No incremental sync, cursor, partition router, or state format is introduced or changed, so there is no per-partition state to migrate and rollback simply drops the new streams.
Behavioral Changes PASS Warning (elevates Risk Level) No rate-limit, retry, backoff, timeout, error-handler, or page-size keyword in the non-comment diff hunks. NoPagination is a new-stream property, not a change to existing request behavior.
Out-of-Scope Changes PASS Skip All changes are under airbyte-integrations/connectors/source-float/** and docs/integrations/sources/float.md.
CI Checks PASS Yes Core checks green on 0a7702a5: Lint source-float Connector, Test source-float Connector, Test source-float Connector [No Creds], `source-float` Connector Test Results, Connector CI Checks Summary, Format Check, Docs / Vale, Docs / MarkDownLint, Build Airbyte Docs, Analyze Python. The failing source-float Progressive Rollout Gate, Connector Active Progressive Rollout Checks Summary, and Validate PR Title runs are excluded by playbook rule (release-management / repo-meta checks, not build/lint/test); each also has a passing run on this SHA.
Live / E2E Tests UNKNOWN Yes Validation is required (new streams → discovery/catalog category), but no /ai-prove-fix conclusion exists on this PR. Not disproven — simply not yet validated, so this is UNKNOWN rather than FAIL.
📚 Evidence Consulted

Evidence

  • Changed files: 7 (manifest.yaml +200, metadata.yaml +1/-1, unit_tests/conftest.py +39, unit_tests/pyproject.toml +20, unit_tests/poetry.lock +3056, unit_tests/test_reports_streams.py +366, docs/integrations/sources/float.md +12)
  • Spec comparison: master vs 0a7702a5 connection_specification.required — identical
  • CI checks: 60+ check-runs on 0a7702a5; all core connector build/lint/test checks successful
  • PR labels: community, connectors/source/float, hyd-review
  • PR description: present (2,089 chars, structured What / How / Review guide / User Impact)
  • Human review comments: 1 (Baz (@bazarnov), unresolved, no author reply); formal PR reviews: none; inline review threads: 0
  • /ai-prove-fix runs: none found
  • Existing bot reviews: prior pr_ai_review marker FAIL for 09f4a034 (superseded by this run)
❓ How to Respond

Resolving a Failing Gate

Each non-PASS gate above lists concrete remediation. Apply it, push, then re-run /ai-review — the bot re-evaluates from the diff, CI check-runs, and /ai-prove-fix results.

Written explanations do not change a gate verdict. If you believe a gate is a false positive or it cannot be remediated as written, request review from a human maintainer and explain the situation to them in a PR comment — a human reviewer can approve or merge over a bot FAIL. Never edit the PR description to argue with the bot; the description is how reviewers learn what the PR does.

@masterpradipg

Copy link
Copy Markdown
Author

Baz (@bazarnov) Thanks for the thorough review and for pushing 0a7702a, I have gone through all
three items and I am happy with every one of them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Development

Successfully merging this pull request may close these issues.

5 participants