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

Fix create PR in release workflow #3528

Merged
merged 6 commits into from
Apr 1, 2024
Merged

Fix create PR in release workflow #3528

merged 6 commits into from
Apr 1, 2024

Conversation

shashank-elastic
Copy link
Contributor

@shashank-elastic shashank-elastic commented Mar 21, 2024

Issues

#3527

Summary

  • Use the supported format of GitHub PR for 3.12
  • Per the 3.12 docs the expected format would be
pr = repo.create_pull(
    base=base_branch,
    head=branch_name,
    title=message,
    body=body,
    maintainer_can_modify=True,
    draft=draft
)

@shashank-elastic shashank-elastic self-assigned this Mar 21, 2024
@botelastic botelastic bot added the python Internal python for the repository label Mar 21, 2024
@shashank-elastic shashank-elastic marked this pull request as draft March 21, 2024 18:55
@shashank-elastic
Copy link
Contributor Author

The workflow was issued to test from private branch on old backport of 8.9 https://github.com/elastic/detection-rules/actions/runs/8380327773/job/22949293741

But the cahnges are not reflected. Checking this further.

@shashank-elastic
Copy link
Contributor Author

The workflow will always point this to the lock-versions commit head so practically this change was not reflected in the workflow run. Should find a way to test this

image

@Mikaayenson
Copy link
Contributor

The workflow will always point this to the lock-versions commit head so practically this change was not reflected in the workflow run. Should find a way to test this

@shashank-elastic You don't need to run the entire workflow to test. You can debug the method locally that's failing (and pass the correct parameters).

python -m detection_rules dev integrations-pr \
    $LOCAL_REPO                                 \
    --github-repo $TARGET_REPO                  \
    --base-branch $TARGET_BRANCH                \
    --assign shashank-elastic                  \
    $DRAFT_ARGS

I recommend setting up a debug session and set breakpoints in the integrations-pr command.

@shashank-elastic
Copy link
Contributor Author

Local Testing

  • Build a test release package
  • Installed go "elastic-package" as a dependancy
  • Issued a local command to create the PR
python -m detection_rules dev integrations-pr ../integrations --base-branch test-dr-release-3527 --assign shashank-elastic

█▀▀▄ ▄▄▄ ▄▄▄ ▄▄▄ ▄▄▄ ▄▄▄ ▄▄▄ ▄▄▄ ▄   ▄      █▀▀▄ ▄  ▄ ▄   ▄▄▄ ▄▄▄
█  █ █▄▄  █  █▄▄ █    █   █  █ █ █▀▄ █      █▄▄▀ █  █ █   █▄▄ █▄▄
█▄▄▀ █▄▄  █  █▄▄ █▄▄  █  ▄█▄ █▄█ █ ▀▄█      █ ▀▄ █▄▄█ █▄▄ █▄▄ ▄▄█

Switched to a new branch 'test-dr-release-3527'
From github.com:elastic/integrations
 * branch                test-dr-release-3527 -> FETCH_HEAD
Switched to a new branch 'detection-rules/8.14.1-beta.1-52868768'
2024/03/26 22:09:53  WARN CommitHash is undefined, in both /Users/shashankks/.elastic-package/version and the compiled binary, config may be out of date.
2024/03/26 22:09:53  WARN reading version file failed: open /Users/shashankks/.elastic-package/latestVersion: no such file or directory
Format the package
Done
Enumerating objects: 11, done.
Counting objects: 100% (11/11), done.
Delta compression using up to 12 threads
Compressing objects: 100% (6/6), done.
Writing objects: 100% (6/6), 558 bytes | 558.00 KiB/s, done.
Total 6 (delta 5), reused 0 (delta 0), pack-reused 0
remote: Resolving deltas: 100% (5/5), completed with 5 local objects.
remote: 
remote: Create a pull request for 'detection-rules/8.14.1-beta.1-52868768' on GitHub by visiting:
remote:      https://github.com/elastic/integrations/pull/new/detection-rules/8.14.1-beta.1-52868768
remote: 
remote: GitHub found 1 vulnerability on elastic/integrations's default branch (1 moderate). To find out more, visit:
remote:      https://github.com/elastic/integrations/security/dependabot/16
remote: 
To github.com:elastic/integrations.git
 * [new branch]          detection-rules/8.14.1-beta.1-52868768 -> detection-rules/8.14.1-beta.1-52868768
PR created:
https://github.com/elastic/integrations/pull/9452
2024/03/26 22:10:00  WARN CommitHash is undefined, in both /Users/shashankks/.elastic-package/version and the compiled binary, config may be out of date.
Format the package
Done
2024/03/26 22:10:01  WARN CommitHash is undefined, in both /Users/shashankks/.elastic-package/version and the compiled binary, config may be out of date.
Lint the package
Done
Enumerating objects: 9, done.
Counting objects: 100% (9/9), done.
Delta compression using up to 12 threads
Compressing objects: 100% (5/5), done.
Writing objects: 100% (5/5), 459 bytes | 459.00 KiB/s, done.
Total 5 (delta 4), reused 0 (delta 0), pack-reused 0
remote: Resolving deltas: 100% (4/4), completed with 4 local objects.
remote: 
remote: GitHub found 1 vulnerability on elastic/integrations's default branch (1 moderate). To find out more, visit:
remote:      https://github.com/elastic/integrations/security/dependabot/16
remote: 
To github.com:elastic/integrations.git
   49d332673..ee2d78ea0  detection-rules/8.14.1-beta.1-52868768 -> detection-rules/8.14.1-beta.1-52868768
(.venv) 
detection-rules on  issue-3527 [$!?] is 📦 v0.1.0 via 🐹 v1.22.1 via 🐍 v3.12.2 (.venv) on ☁️  shashank.suryanarayana@elastic.co took 21s 
❯ 

PR successfully created - elastic/integrations#9452

@shashank-elastic shashank-elastic marked this pull request as ready for review March 26, 2024 16:48
@@ -675,7 +675,7 @@ def elastic_pkg(*args):
None
""") # noqa: E501

pr = repo.create_pull(message, body, base_branch, branch_name, maintainer_can_modify=True, draft=draft)
pr = repo.create_pull(title=message, body=body, base=base_branch, head=branch_name, maintainer_can_modify=True, draft=draft)
Copy link
Contributor

Choose a reason for hiding this comment

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

Note: create pull is called in a couple places in the repo.

detection_rules/etc/packages.yml Outdated Show resolved Hide resolved
@eric-forte-elastic
Copy link
Contributor

Just to double check, should the other call be updated to match this one as well?

    pr = repo.create_pull(title, body, base=kwargs["base_branch"], head=branch_name, maintainer_can_modify=True,
                          draft=draft)

@shashank-elastic
Copy link
Contributor Author

Just to double check, should the other call be updated to match this one as well?

    pr = repo.create_pull(title, body, base=kwargs["base_branch"], head=branch_name, maintainer_can_modify=True,
                          draft=draft)

Honestly, that was not scoped here, so if we intend to that we would need to test the workflow kibana-pr.
This would mean understanding what it takes to create a kibana-pr and test. But this fix needs to go before next week DR release else we have the release automation workflow failures like we saw in 8.12.2 release.
If we can decouple the fixes and send this in given the time frame we are operating it would be beneficial!

Also looking at this post title and body are not the "required parameters" so may be we will no hit the issue like in integrations_pr where even the required parameters such as "base_branch" and "branch_name" are also not aligned as per 3.12 requirements.

cc @eric-forte-elastic

Copy link
Contributor

@eric-forte-elastic eric-forte-elastic left a comment

Choose a reason for hiding this comment

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

Thanks for the explanation! Also looks like the kibana pr dev command is no longer used in release workflows so not required to fix for the release process.

LGTM 👍

@Mikaayenson
Copy link
Contributor

Mikaayenson commented Mar 26, 2024

Concur, with @eric-forte-elastic. The other references should be updated.

OR immediately in a separate issue/PR, delete:

  • def add_kibana_git_args(f):
  • def kibana_commit(
  • def kibana_pr(

if we no longer do this.

@shashank-elastic
Copy link
Contributor Author

@Mikaayenson The deprectaion is handled in #3552.
Can we push this async.

Copy link
Contributor

@Mikaayenson Mikaayenson left a comment

Choose a reason for hiding this comment

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

LGTM

@shashank-elastic shashank-elastic merged commit 8b215ea into main Apr 1, 2024
14 checks passed
@shashank-elastic shashank-elastic deleted the issue-3527 branch April 1, 2024 15:47
protectionsmachine pushed a commit that referenced this pull request Apr 1, 2024
protectionsmachine pushed a commit that referenced this pull request Apr 1, 2024
protectionsmachine pushed a commit that referenced this pull request Apr 1, 2024
protectionsmachine pushed a commit that referenced this pull request Apr 1, 2024
protectionsmachine pushed a commit that referenced this pull request Apr 1, 2024
protectionsmachine pushed a commit that referenced this pull request Apr 1, 2024
protectionsmachine pushed a commit that referenced this pull request Apr 1, 2024
protectionsmachine pushed a commit that referenced this pull request Apr 1, 2024
protectionsmachine pushed a commit that referenced this pull request Apr 1, 2024
protectionsmachine pushed a commit that referenced this pull request Apr 1, 2024
protectionsmachine pushed a commit that referenced this pull request Apr 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport: auto python Internal python for the repository
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] Detection Rules Release Workflow is failing to successfully create and Integrations PR
3 participants