Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

airbyte-ci: bump tag, update readme, remove 'nightly_builds' #32806

Conversation

postamar
Copy link
Contributor

@postamar postamar commented Nov 24, 2023

This PR gets rid of the "nightly_builds" CI_CONTEXT value.

This PR changes the behaviour of --modified when CI_CONTEXT is "nightly_builds" or "weekly_builds".

Presently, when --modified and either of those CI_CONTEXT values are set, airbyte-ci determines which connectors have been modified by looking only at which connectors were changed in whatever happens to be the HEAD commit on the master branch at that time.
This is certainly not what anyone wants.

This PR changes this to look at the connectors changed in HEAD and all of its ancestors up until a certain date.
This cutoff date is '1 day ago' for nightlies and '1 week ago' for weeklies.

@postamar postamar requested a review from a team November 24, 2023 17:07
Copy link

vercel bot commented Nov 24, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
airbyte-docs ⬜️ Ignored (Inspect) Visit Preview Nov 27, 2023 4:12pm

@postamar postamar force-pushed the postamar_gt_11-24-airbyte-ci_correct_time-travelling_behaviour_for_cron_jobs branch from 20b2931 to f348a0f Compare November 24, 2023 17:15
@postamar postamar changed the title airbyte-ci: correct time-travelling behaviour for cron jobs airbyte-ci: correct time-travelling behaviour for --modified Nov 24, 2023
@postamar postamar force-pushed the postamar_gt_11-24-airbyte-ci_correct_time-travelling_behaviour_for_cron_jobs branch from f348a0f to ba268a8 Compare November 24, 2023 18:13
Copy link
Contributor

@bnchrch bnchrch left a comment

Choose a reason for hiding this comment

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

LGTM! 💎

async def get_modified_files_since(current_git_branch: str, current_git_revision: str, since: str, is_local: bool = True) -> Set[str]:
# TODO: implement
return await get_modified_files_in_commit(current_git_branch, current_git_revision, is_local)
if is_local:
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for filling out the helper!

@@ -74,9 +72,30 @@ async def get_modified_files_in_commit(current_git_branch: str, current_git_revi
return await get_modified_files_in_commit_remote(current_git_branch, current_git_revision)


def get_modified_files_since_local(current_git_revision: str, since: str) -> Set[str]:
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd appreciate since to be a datetime / date object and make the function perform a strftime to cast it to string.

Comment on lines 75 to 83
def get_modified_files_since_local(current_git_revision: str, since: str) -> Set[str]:
airbyte_repo = git.Repo()
diffed_git_revision = airbyte_repo.git.log(current_git_revision, f"--before='{since}'", "--max-count=1", "--pretty=%H").strip()
return get_modified_files_local(current_git_revision, diffed_git_revision)


async def get_modified_files_since_remote(current_git_branch: str, current_git_revision: str, since: str) -> Set[str]:
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't find these two helpers being used? Or am I missing coffee? If they're not used yet I'm not sure the version bump should happen in this branch.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're missing coffee. In any case I just got rid of them. I'm going to merge this stack just to fix the stacked pull request behaviour.

@postamar postamar force-pushed the postamar_gt_11-24-airbyte-ci_correct_type_annotation_deterministic_transform_strs_to_paths_behaviour branch from e9c2a7d to 850daee Compare November 27, 2023 15:26
@postamar postamar force-pushed the postamar_gt_11-24-airbyte-ci_correct_time-travelling_behaviour_for_cron_jobs branch from ba268a8 to 7b5e8e3 Compare November 27, 2023 15:26
@postamar postamar force-pushed the postamar_gt_11-24-airbyte-ci_correct_type_annotation_deterministic_transform_strs_to_paths_behaviour branch from e97b5fd to 54307a1 Compare November 27, 2023 15:42
@postamar postamar force-pushed the postamar_gt_11-24-airbyte-ci_correct_time-travelling_behaviour_for_cron_jobs branch from 541c18d to 210b58e Compare November 27, 2023 15:42
@postamar postamar changed the title airbyte-ci: correct time-travelling behaviour for --modified airbyte-ci: bump tag, update readme, remove 'nightly_builds' Nov 27, 2023
@postamar postamar force-pushed the postamar_gt_11-24-airbyte-ci_correct_type_annotation_deterministic_transform_strs_to_paths_behaviour branch from 138055e to 71c4cb7 Compare November 27, 2023 16:08
@postamar postamar force-pushed the postamar_gt_11-24-airbyte-ci_correct_time-travelling_behaviour_for_cron_jobs branch from 11d0983 to d64321e Compare November 27, 2023 16:08
@postamar postamar changed the base branch from postamar_gt_11-24-airbyte-ci_correct_type_annotation_deterministic_transform_strs_to_paths_behaviour to master November 27, 2023 16:41
@postamar
Copy link
Contributor Author

/approve-and-merge reason="blocked by docker pull rate limit"

@octavia-approvington
Copy link
Contributor

Myomoto says it looks good
thats a niiice

@octavia-approvington octavia-approvington merged commit 8c71120 into master Nov 27, 2023
19 of 20 checks passed
@octavia-approvington octavia-approvington deleted the postamar_gt_11-24-airbyte-ci_correct_time-travelling_behaviour_for_cron_jobs branch November 27, 2023 16:42
postamar added a commit that referenced this pull request Nov 27, 2023
octavia-approvington pushed a commit that referenced this pull request Nov 27, 2023
postamar added a commit that referenced this pull request Nov 27, 2023
@postamar postamar mentioned this pull request Nov 27, 2023
postamar added a commit that referenced this pull request Nov 28, 2023
Co-authored-by: postamar <postamar@users.noreply.github.com>
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.

None yet

4 participants