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.

Sign up for GitHub

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

Jump to bottom

Set global cache_dir_lock #4055

Merged
merged 2 commits into from
Mar 8, 2024
Merged

Set global cache_dir_lock #4055

merged 2 commits into from
Mar 8, 2024

Conversation

guppy0130
Copy link
Contributor

  • in initialize_options, we need to update the globally-scoped
    cache_dir_lock so that later on, main can release the lock and
    cleanup the lockfile.

Fixes #4030

@guppy0130 guppy0130 requested a review from a team as a code owner March 2, 2024 18:46
@guppy0130 guppy0130 requested review from ssbarnea and shatakshiiii and removed request for a team March 2, 2024 18:46
@github-actions github-actions bot added the bug label Mar 2, 2024
@guppy0130
Copy link
Contributor Author

guppy0130 commented Mar 2, 2024 •
edited
Loading

This is the smallest possible changeset that I believe resolves the issue. Linters are not happy with the usage of global though.

There's a couple alternative implementations that I believe would be better:

@audgirka
Copy link
Contributor

audgirka commented Mar 7, 2024 •
edited
Loading

This is the smallest possible changeset that I believe resolves the issue. Linters are not happy with the usage of global though.

There's a couple alternative implementations that I believe would be better:

* storing `cache_dir_lock` in the `options` object
  
  * I don't know why `cache_dir_lock` was removed from the `options` object in [Address parallel install errors with ansible-galaxy on GHA #3585](https://github.com/ansible/ansible-lint/pull/3585) - lacking context for that PR

* `initialize_options` could return the `LockFile` object and we perform the lock acquisition and release in `main`

@ssbarnea can you please confirm for the first point which talks about removing cache_dir_lock from options object.

I like the second option for initialize_options to return FileLock or None which allows us to use cache_dir_lock as a local variable in main() and it will not be a global variable anymore

@audgirka audgirka enabled auto-merge (squash) March 8, 2024 11:07
@audgirka audgirka disabled auto-merge March 8, 2024 11:10
@audgirka audgirka merged commit 36e2674 into ansible:main Mar 8, 2024
23 checks passed
@guppy0130 guppy0130 deleted the fix/cache_dir_lock branch March 10, 2024 22:15
nrdufour added a commit to nrdufour/home-ops that referenced this pull request Mar 14, 2024
@nrdufour
chore(deps): update dependency ansible-lint to v24.2.1 (#434)
fcd643b 
This PR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [ansible-lint](https://github.com/ansible/ansible-lint) ([changelog](https://github.com/ansible/ansible-lint/releases)) | patch | `==24.2.0` -> `==24.2.1` |

---

### Release Notes

<details>
<summary>ansible/ansible-lint (ansible-lint)</summary>

### [`v24.2.1`](https://github.com/ansible/ansible-lint/releases/tag/v24.2.1)

[Compare Source](ansible/ansible-lint@v24.2.0...v24.2.1)

#### Bugfixes

-   Fix error suppression when syntax is incorrect ([#&#8203;4026](ansible/ansible-lint#4026)) [@&#8203;audgirka](https://github.com/audgirka)
-   Set global `cache_dir_lock` ([#&#8203;4055](ansible/ansible-lint#4055)) [@&#8203;guppy0130](https://github.com/guppy0130)
-   \[role-name] Handle string role dependencies ([#&#8203;4054](ansible/ansible-lint#4054)) [@&#8203;corubba](https://github.com/corubba)
-   Support for upcoming Ubuntu 24.04 and Fedora 40 ([#&#8203;4051](ansible/ansible-lint#4051)) [@&#8203;mafalb](https://github.com/mafalb)
-   Update documentation for syntax-check\[unknown-module] ([#&#8203;4049](ansible/ansible-lint#4049)) [@&#8203;ssbarnea](https://github.com/ssbarnea)
-   Add OpenWRT 23.05 to valid versions ([#&#8203;4041](ansible/ansible-lint#4041)) [@&#8203;jonkerj](https://github.com/jonkerj)
-   Update task name in `notify` for a task against `name[casing]` error ([#&#8203;4038](ansible/ansible-lint#4038)) [@&#8203;audgirka](https://github.com/audgirka)
-   accept yum history and info as valid commands ([#&#8203;4025](ansible/ansible-lint#4025)) [@&#8203;konstruktoid](https://github.com/konstruktoid)
-   Add attributes for role argument specs ([#&#8203;4018](ansible/ansible-lint#4018)) [@&#8203;felixfontein](https://github.com/felixfontein)

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4yNDEuMSIsInVwZGF0ZWRJblZlciI6IjM3LjI0MS4xIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9-->

Co-authored-by: Nicolas Dufour <nrdufour@gmail.com>
Reviewed-on: https://git.internal/nrdufour/home-ops/pulls/434
Co-authored-by: Renovate <renovate@ptinem.io>
Co-committed-by: Renovate <renovate@ptinem.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@audgirka audgirka audgirka approved these changes

@ssbarnea ssbarnea Awaiting requested review from ssbarnea ssbarnea is a code owner automatically assigned from ansible/devtools

@shatakshiiii shatakshiiii Awaiting requested review from shatakshiiii shatakshiiii is a code owner automatically assigned from ansible/devtools

Assignees
No one assigned
Labels
bug
Projects
🧰 devtools project board
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

cache_dir_lock in main is always None, so the lockfile never gets cleaned up
2 participants
@guppy0130 @audgirka