Skip to content

fix: regenerate lockfile by default on extras-integration (FXC-4769)#3202

Merged
daquinteroflex merged 1 commit intodevelopfrom
dario/regenerate_lockfile
Jan 26, 2026
Merged

fix: regenerate lockfile by default on extras-integration (FXC-4769)#3202
daquinteroflex merged 1 commit intodevelopfrom
dario/regenerate_lockfile

Conversation

@daquinteroflex
Copy link
Collaborator

@daquinteroflex daquinteroflex commented Jan 26, 2026

Note

Ensures dependencies are freshly resolved during CI for extras integration tests.

  • Adds regenerate-lockfile step (poetry update --lock) after CodeArtifact auth and before project install in both integration-tests-basic and integration-tests-full jobs

Written by Cursor Bugbot for commit dc32360. This will update automatically on new commits. Configure here.

Greptile Overview

Greptile Summary

Adds poetry update --lock step to both integration-tests-basic and integration-tests-full jobs to regenerate the lockfile before installing dependencies, ensuring tests run against freshly resolved dependencies from CodeArtifact.

Key concerns:

  • Running poetry update --lock on every CI run means tests execute against different dependency versions than the committed poetry.lock, which defeats the purpose of lockfiles and can mask version-specific issues
  • The regenerated lockfile is never committed, creating a mismatch between CI and local/production environments
  • Adds dependency resolution overhead to every test run (typically 30-60+ seconds)
  • The PR description mentions "freshly resolved dependencies" as the goal, but this approach trades reproducibility for freshness

Alternative approaches to consider:

  • Use poetry lock --check to validate the existing lockfile is up-to-date
  • Only regenerate when the lockfile is missing or explicitly requested via workflow input
  • Run this workflow separately as a scheduled dependency update check rather than on every integration test

Confidence Score: 2/5

  • This PR introduces a significant change to CI behavior that undermines lockfile reproducibility
  • While the change is syntactically correct and won't cause immediate failures, it fundamentally changes how dependencies are tested in CI by regenerating the lockfile on every run. This means CI tests run against different versions than what's committed in the repository, defeating the purpose of having a lockfile. The approach trades reproducibility for freshness, which could mask dependency-related bugs that would appear in production. A score of 2 reflects that this needs reconsideration of the approach.
  • Pay close attention to .github/workflows/tidy3d-extras-python-client-tests-integration.yml - the lockfile regeneration strategy should be reconsidered

Important Files Changed

Filename Overview
.github/workflows/tidy3d-extras-python-client-tests-integration.yml Added regenerate-lockfile step using poetry update --lock to both integration test jobs; this regenerates dependencies on every CI run which may mask dependency issues and increase build times

Sequence Diagram

sequenceDiagram
    participant GHA as GitHub Actions
    participant AWS as AWS CodeArtifact
    participant Poetry as Poetry
    participant Tests as Integration Tests

    GHA->>GHA: Checkout code
    GHA->>GHA: Setup Python & Poetry
    GHA->>AWS: Configure AWS credentials
    GHA->>AWS: Get CodeArtifact auth token
    AWS-->>GHA: Return auth token
    GHA->>Poetry: Configure CodeArtifact credentials
    Note over Poetry: NEW STEP
    GHA->>Poetry: poetry update --lock
    Poetry->>AWS: Fetch latest dependency versions
    AWS-->>Poetry: Return dependency metadata
    Poetry->>Poetry: Regenerate poetry.lock
    Note over Poetry: Lock file now has fresh versions
    GHA->>Poetry: poetry install -E extras -E dev
    Poetry->>AWS: Download dependencies
    AWS-->>Poetry: Return packages
    Poetry->>GHA: Installation complete
    GHA->>Tests: Run integration tests
    Tests-->>GHA: Test results
Loading

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

4 files reviewed, 4 comments

Edit Code Review Agent Settings | Greptile

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.

@github-actions
Copy link
Contributor

Diff Coverage

Diff: origin/develop...HEAD, staged and unstaged changes

No lines with coverage information in this diff.

@daquinteroflex daquinteroflex changed the title fix: regenerate lockfile by default on extras-integration fix: regenerate lockfile by default on extras-integration (FXC-4769) Jan 26, 2026
@daquinteroflex
Copy link
Collaborator Author

So this is intentional. The issue is that lock-filing is not really applicable with tidy3d-extras until we have a proper daily versioning flow, which we want to enable but right now there is no bandwidth. So this is a temporary change to verify the integration tests always run with the latest tidy3d-extras without needing to manually regenerate the lockfile.

@daquinteroflex daquinteroflex added this pull request to the merge queue Jan 26, 2026
@daquinteroflex daquinteroflex removed this pull request from the merge queue due to a manual request Jan 26, 2026
@daquinteroflex daquinteroflex added this pull request to the merge queue Jan 26, 2026
Merged via the queue into develop with commit 97a2020 Jan 26, 2026
238 of 282 checks passed
@daquinteroflex daquinteroflex deleted the dario/regenerate_lockfile branch January 26, 2026 15:43
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.

2 participants