Skip to content

Conversation

aaronsteers
Copy link
Contributor

fix: Replace pendulum with ab_datetime_now() and fix linting issues

This PR targets the following PR:


Summary

Fixes CI failures in PR #725 that were blocking the stream preview functionality. The failures were caused by:

  1. Missing pendulum imports - Three locations in airbyte/progress.py used pendulum.now().format() without importing pendulum
  2. Linting violations - Private member access warning and code formatting issues
  3. Line length violation - One line exceeded the 100 character limit

Key Changes:

  • Replaced pendulum.now().format('HH:mm:ss') with ab_datetime_now().strftime('%H:%M:%S') in 3 locations (following existing codebase pattern)
  • Added # noqa: SLF001 comment to suppress intentional private member access warning
  • Fixed line length violation by splitting long f-string
  • Applied ruff formatting to both modified files

Review & Testing Checklist for Human

  • Verify datetime format equivalence: Confirm that ab_datetime_now().strftime('%H:%M:%S') produces identical output to pendulum.now().format('HH:mm:ss')
  • Test stream preview functionality end-to-end: Run the actual source preview features to ensure no functional regressions
  • Validate replacement pattern: Confirm that using ab_datetime_now() aligns with the intended "whenever" replacement mentioned in the original context
  • Run broader test suite: Execute additional test suites beyond the single failing test to check for edge cases

Recommended Test Plan:

  1. Run the specific failing test: poetry run pytest tests/unit_tests/test_lowcode_connectors.py::test_nocode_execution -v
  2. Test a real source with preview functionality to verify timestamps display correctly
  3. Run the full unit test suite to check for regressions: poetry run pytest tests/unit_tests/ -v

Diagram

%%{ init : { "theme" : "default" }}%%
graph TD
    source["airbyte/sources/base.py<br/>(Minor Edit)"]:::minor-edit
    progress["airbyte/progress.py<br/>(Major Edit)"]:::major-edit  
    test["tests/unit_tests/test_lowcode_connectors.py<br/>(Context)"]:::context
    datetime["airbyte_cdk.utils.datetime_helpers<br/>(Context)"]:::context
    
    source -->|"calls _log_sync_cancel()"| progress
    progress -->|"imports ab_datetime_now"| datetime
    test -->|"executes source.read()"| source
    progress -->|"formats timestamps"| progress
    
    subgraph Legend
        L1["Major Edit"]:::major-edit
        L2["Minor Edit"]:::minor-edit  
        L3["Context/No Edit"]:::context
    end

    classDef major-edit fill:#90EE90
    classDef minor-edit fill:#87CEEB
    classDef context fill:#FFFFFF
Loading

Notes

  • Local verification passed: All ruff formatting, linting, mypy type checking, and the specific failing test now pass locally
  • Assumption made: Used ab_datetime_now() pattern based on existing codebase usage, though original context mentioned "whenever" replacement
  • Scope limitation: Only tested the specific failing test case locally, not the full test suite
  • Session context: Requested by AJ Steers (@aaronsteers) - Session: https://app.devin.ai/sessions/6e24d9fe30ab470187ca5eb06f13ebfe

The changes are mechanical replacements following established patterns in the codebase, but human verification of the datetime format equivalence and broader testing is recommended to ensure no subtle regressions.

- Replace pendulum.now().format() calls with ab_datetime_now().strftime() in progress.py
- Add noqa comment for intentional private member access in sources/base.py
- Fix line length violation in progress.py
- Apply ruff formatting to both files

Fixes CI failures in stream preview functionality.

Co-Authored-By: AJ Steers <[email protected]>
@Copilot Copilot AI review requested due to automatic review settings July 31, 2025 06:35
Copy link
Contributor

Original prompt from AJ Steers
@Devin - Create a PR targeting this one: <https://github.com/airbytehq/PyAirbyte/pull/725>

See if you can resolve the CI test failures.

Copy link
Contributor

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

Copy link

👋 Greetings, Airbyte Team Member!

Here are some helpful tips and reminders for your convenience.

Testing This PyAirbyte Version

You can test this version of PyAirbyte using the following:

# Run PyAirbyte CLI from this branch:
uvx --from 'git+https://github.com/airbytehq/PyAirbyte.git@devin/1753943347-fix-ci-failures' pyairbyte --help

# Install PyAirbyte from this branch for development:
pip install 'git+https://github.com/airbytehq/PyAirbyte.git@devin/1753943347-fix-ci-failures'

Helpful Resources

PR Slash Commands

Airbyte Maintainers can execute the following slash commands on your PR:

  • /fix-pr - Fixes most formatting and linting issues
  • /poetry-lock - Updates poetry.lock file
  • /test-pr - Runs tests with the updated PyAirbyte

Community Support

Questions? Join the #pyairbyte channel in our Slack workspace.

📝 Edit this welcome message.

Copy link
Contributor

@Copilot 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 fixes CI failures in PR #725 by replacing missing pendulum imports with the ab_datetime_now() utility function and addressing linting violations. The changes enable the stream preview functionality to pass CI checks.

  • Replaced three instances of pendulum.now().format('HH:mm:ss') with ab_datetime_now().strftime('%H:%M:%S')
  • Fixed linting issues including private member access warning and line length violations
  • Applied consistent code formatting to improve readability

Reviewed Changes

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

File Description
airbyte/sources/base.py Added noqa comment for intentional private member access and reformatted code for line length compliance
airbyte/progress.py Replaced pendulum datetime calls with ab_datetime_now() and fixed string formatting for readability

Copy link

github-actions bot commented Jul 31, 2025

PyTest Results (Fast Tests Only, No Creds)

256 tests  +212   256 ✅ +213   3m 27s ⏱️ + 3m 23s
  1 suites ±  0     0 💤 ±  0 
  1 files   ±  0     0 ❌  -   1 

Results for commit 781090d. ± Comparison against base commit fff994e.

♻️ This comment has been updated with latest results.

@aaronsteers aaronsteers changed the title fix: Replace pendulum with ab_datetime_now() and fix linting issues (do not merge) fix: Replace pendulum with ab_datetime_now() and fix linting issues Jul 31, 2025
@aaronsteers
Copy link
Contributor Author

Copilot pointed out we have some missing import statements.

Copy link

github-actions bot commented Jul 31, 2025

PyTest Results (Full)

318 tests  ±0   304 ✅ +52   17m 23s ⏱️ + 4m 42s
  1 suites ±0    14 💤 ± 0 
  1 files   ±0     0 ❌  - 52 

Results for commit 781090d. ± Comparison against base commit fff994e.

♻️ This comment has been updated with latest results.

@aaronsteers aaronsteers merged commit f0d90f7 into aj/feat/stream-previews Jul 31, 2025
18 checks passed
@aaronsteers aaronsteers deleted the devin/1753943347-fix-ci-failures branch July 31, 2025 18:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant