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

Reapply (Git-6) Use new Git Backend in Entire auto_tick flow #3124

Merged
merged 2 commits into from
Nov 11, 2024

Conversation

ytausch
Copy link
Contributor

@ytausch ytausch commented Nov 11, 2024

Description:

This PR reapplies the revert done in #3024.

The error was observed because the HTTP Basic Auth token needs to be base64 encoded.

c54001a applies the necessary fix.

Checklist:

  • Pydantic model updated or no update needed

Cross-refs, links to issues, etc:

Cc @pavelzw

Copy link
Contributor

@beckermr beckermr left a comment

Choose a reason for hiding this comment

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

LGTM!

@beckermr beckermr merged commit d0adcb3 into regro:main Nov 11, 2024
5 checks passed
Copy link

codecov bot commented Nov 11, 2024

Codecov Report

Attention: Patch coverage is 92.81437% with 36 lines in your changes missing coverage. Please review.

Project coverage is 77.46%. Comparing base (c77cf4a) to head (c54001a).
Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
conda_forge_tick/auto_tick.py 15.78% 16 Missing ⚠️
conda_forge_tick/git_utils.py 89.16% 13 Missing ⚠️
tests/test_git_utils.py 98.01% 7 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3124      +/-   ##
==========================================
+ Coverage   76.60%   77.46%   +0.85%     
==========================================
  Files         124      124              
  Lines       13017    13280     +263     
==========================================
+ Hits         9972    10287     +315     
+ Misses       3045     2993      -52     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ytausch ytausch deleted the reapply-git-backend-6 branch November 11, 2024 15:35
@beckermr
Copy link
Contributor

@ytausch we need to revert this one again: See the errors in this job:

https://github.com/regro/cf-scripts/actions/runs/11783101890/job/32819561459

@ytausch
Copy link
Contributor Author

ytausch commented Nov 11, 2024

@ytausch we need to revert this one again: See the errors in this job:

https://github.com/regro/cf-scripts/actions/runs/11783101890/job/32819561459

I think this is not a functional bug but just confusing log output. Very likely the message is generated by these lines:

try:
logger.debug(
f"Trying to checkout branch {new_branch} without creating a new branch"
)
self.checkout_branch(target_dir, new_branch)
except GitCliError:
logger.debug(
f"It seems branch {new_branch} does not exist. Creating it.",
)
self.checkout_new_branch(target_dir, new_branch, start_point=base_branch)

(The bot tries to checkout a branch to see if it already exists.)

I can suppress stdout and stderr output for these speculative git invocations, WDYT?

@beckermr
Copy link
Contributor

No there is a bug. See here:

  2024-11-11 17:21:03,988 ERROR    conda_forge_tick.auto_tick || NON GITHUB ERROR
  Traceback (most recent call last):
    File "/home/runner/work/cf-scripts/cf-scripts/cf-scripts/conda_forge_tick/auto_tick.py", line 720, in _run_migrator_on_feedstock_branch
      migrator_uid, pr_json = run_with_tmpdir(
                              ^^^^^^^^^^^^^^^^
    File "/home/runner/work/cf-scripts/cf-scripts/cf-scripts/conda_forge_tick/auto_tick.py", line 451, in run_with_tmpdir
      return run(
             ^^^^
    File "/home/runner/work/cf-scripts/cf-scripts/cf-scripts/conda_forge_tick/auto_tick.py", line 582, in run
      pr_data = git_backend.create_pull_request(
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    File "/home/runner/work/cf-scripts/cf-scripts/cf-scripts/conda_forge_tick/git_utils.py", line 941, in create_pull_request
      header_fields = {
                      ^
    File "/home/runner/work/cf-scripts/cf-scripts/cf-scripts/conda_forge_tick/git_utils.py", line 942, in <dictcomp>
      k: self._github3_session.last_response_headers[k]
         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^
    File "/home/runner/micromamba/envs/cf-scripts/lib/python3.11/site-packages/requests/structures.py", line 52, in __getitem__
      return self._store[key.lower()][1]
             ~~~~~~~~~~~^^^^^^^^^^^^^
  KeyError: 'last-modified'

@ytausch
Copy link
Contributor Author

ytausch commented Nov 11, 2024

Just verified: The GitHub API returns the Last-Modified header only if we get a PR with GET, and not when creating one with POST, although the response is otherwise identical. Should not be too hard to fix.

@ytausch ytausch mentioned this pull request Nov 11, 2024
1 task
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.

3 participants