Skip to content

Conversation

@sanderr
Copy link
Contributor

@sanderr sanderr commented May 15, 2024

Description

#7612 made the autostarted agent manager take full authority over autostarted agents. However, it was decided that it should not start an autostarted agent process if a session is already active for all required agents, even if that session is not with an autostarted process.

This pull request restores that behavior. It adds back two utility methods without change that were dropped in #7612, and its associated test case. It adds a new guard in _ensure_agents and adds a test case (parametrization of an existing one) for the intended behavior. The new test case has been verified to fail on #7612, and to succeed on the commit that precedes it (ensuring backwards compatibility).

Self Check:

Strike through any lines that are not applicable (~~line~~) then check the box

  • Attached issue to pull request
  • Changelog entry
  • Type annotations are present
  • Code is clear and sufficiently documented
  • No (preventable) type errors (check using make mypy or make mypy-diff)
  • Sufficient test cases (reproduces the bug/tests the requested feature)
  • Correct, in line with design
  • End user documentation is included or an issue is created for end-user documentation (add ref to issue here: )
  • If this PR fixes a race condition in the test suite, also push the fix to the relevant stable branche(s) (see test-fixes for more info)

@sanderr sanderr requested a review from wouterdb May 15, 2024 09:14
@sanderr sanderr requested a review from wouterdb May 15, 2024 10:00
@sanderr sanderr added the merge-tool-ready This ticket is ready to be merged in label May 15, 2024
@inmantaci
Copy link
Contributor

Processing this pull request

inmantaci pushed a commit that referenced this pull request May 15, 2024
…tostarted (PR #7628)

# Description

#7612 made the autostarted agent manager take full authority over autostarted agents. However, it was decided that it should not start an autostarted agent process if a session is already active for all required agents, even if that session is not with an autostarted process.

This pull request restores that behavior. It adds back two utility methods without change that were dropped in #7612, and its associated test case. It adds a new guard in `_ensure_agents` and adds a test case (parametrization of an existing one) for the intended behavior. The new test case has been verified to fail on #7612, and to succeed on the commit that precedes it (ensuring backwards compatibility).

# Self Check:

Strike through any lines that are not applicable (`~~line~~`) then check the box

- [x] ~~Attached issue to pull request~~
- [x] Changelog entry
- [x] Type annotations are present
- [x] Code is clear and sufficiently documented
- [x] No (preventable) type errors (check using make mypy or make mypy-diff)
- [x] Sufficient test cases (reproduces the bug/tests the requested feature)
- [x] Correct, in line with design
- [x] ~~End user documentation is included or an issue is created for end-user documentation (add ref to issue here: )~~
- [x] ~~If this PR fixes a race condition in the test suite, also push the fix to the relevant stable branche(s) (see [test-fixes](https://internal.inmanta.com/development/core/tasks/build-master.html#test-fixes) for more info)~~
inmantaci pushed a commit that referenced this pull request May 15, 2024
…tostarted (PR #7628)

# Description

#7612 made the autostarted agent manager take full authority over autostarted agents. However, it was decided that it should not start an autostarted agent process if a session is already active for all required agents, even if that session is not with an autostarted process.

This pull request restores that behavior. It adds back two utility methods without change that were dropped in #7612, and its associated test case. It adds a new guard in `_ensure_agents` and adds a test case (parametrization of an existing one) for the intended behavior. The new test case has been verified to fail on #7612, and to succeed on the commit that precedes it (ensuring backwards compatibility).

# Self Check:

Strike through any lines that are not applicable (`~~line~~`) then check the box

- [x] ~~Attached issue to pull request~~
- [x] Changelog entry
- [x] Type annotations are present
- [x] Code is clear and sufficiently documented
- [x] No (preventable) type errors (check using make mypy or make mypy-diff)
- [x] Sufficient test cases (reproduces the bug/tests the requested feature)
- [x] Correct, in line with design
- [x] ~~End user documentation is included or an issue is created for end-user documentation (add ref to issue here: )~~
- [x] ~~If this PR fixes a race condition in the test suite, also push the fix to the relevant stable branche(s) (see [test-fixes](https://internal.inmanta.com/development/core/tasks/build-master.html#test-fixes) for more info)~~
@inmantaci
Copy link
Contributor

Merged into branches master in 9a07ce3

inmantaci pushed a commit that referenced this pull request May 15, 2024
…tostarted (PR #7628)

# Description

#7612 made the autostarted agent manager take full authority over autostarted agents. However, it was decided that it should not start an autostarted agent process if a session is already active for all required agents, even if that session is not with an autostarted process.

This pull request restores that behavior. It adds back two utility methods without change that were dropped in #7612, and its associated test case. It adds a new guard in `_ensure_agents` and adds a test case (parametrization of an existing one) for the intended behavior. The new test case has been verified to fail on #7612, and to succeed on the commit that precedes it (ensuring backwards compatibility).

# Self Check:

Strike through any lines that are not applicable (`~~line~~`) then check the box

- [x] ~~Attached issue to pull request~~
- [x] Changelog entry
- [x] Type annotations are present
- [x] Code is clear and sufficiently documented
- [x] No (preventable) type errors (check using make mypy or make mypy-diff)
- [x] Sufficient test cases (reproduces the bug/tests the requested feature)
- [x] Correct, in line with design
- [x] ~~End user documentation is included or an issue is created for end-user documentation (add ref to issue here: )~~
- [x] ~~If this PR fixes a race condition in the test suite, also push the fix to the relevant stable branche(s) (see [test-fixes](https://internal.inmanta.com/development/core/tasks/build-master.html#test-fixes) for more info)~~
@inmantaci inmantaci closed this May 15, 2024
@inmantaci inmantaci deleted the ensure-agents-do-not-start-when-active-session branch May 15, 2024 10:25
@inmantaci
Copy link
Contributor

Processing #7630.

inmantaci pushed a commit that referenced this pull request May 15, 2024
…tostarted (PR #7628)

Pull request opened by the merge tool on behalf of #7628
@inmantaci
Copy link
Contributor

Processing #7629.

inmantaci pushed a commit that referenced this pull request May 15, 2024
…tostarted (PR #7628)

Pull request opened by the merge tool on behalf of #7628
arnaudsjs pushed a commit that referenced this pull request May 23, 2024
…tostarted (PR #7628)

# Description

#7612 made the autostarted agent manager take full authority over autostarted agents. However, it was decided that it should not start an autostarted agent process if a session is already active for all required agents, even if that session is not with an autostarted process.

This pull request restores that behavior. It adds back two utility methods without change that were dropped in #7612, and its associated test case. It adds a new guard in `_ensure_agents` and adds a test case (parametrization of an existing one) for the intended behavior. The new test case has been verified to fail on #7612, and to succeed on the commit that precedes it (ensuring backwards compatibility).

# Self Check:

Strike through any lines that are not applicable (`~~line~~`) then check the box

- [x] ~~Attached issue to pull request~~
- [x] Changelog entry
- [x] Type annotations are present
- [x] Code is clear and sufficiently documented
- [x] No (preventable) type errors (check using make mypy or make mypy-diff)
- [x] Sufficient test cases (reproduces the bug/tests the requested feature)
- [x] Correct, in line with design
- [x] ~~End user documentation is included or an issue is created for end-user documentation (add ref to issue here: )~~
- [x] ~~If this PR fixes a race condition in the test suite, also push the fix to the relevant stable branche(s) (see [test-fixes](https://internal.inmanta.com/development/core/tasks/build-master.html#test-fixes) for more info)~~
arnaudsjs pushed a commit that referenced this pull request May 23, 2024
…tostarted (PR #7628)

# Description

#7612 made the autostarted agent manager take full authority over autostarted agents. However, it was decided that it should not start an autostarted agent process if a session is already active for all required agents, even if that session is not with an autostarted process.

This pull request restores that behavior. It adds back two utility methods without change that were dropped in #7612, and its associated test case. It adds a new guard in `_ensure_agents` and adds a test case (parametrization of an existing one) for the intended behavior. The new test case has been verified to fail on #7612, and to succeed on the commit that precedes it (ensuring backwards compatibility).

# Self Check:

Strike through any lines that are not applicable (`~~line~~`) then check the box

- [x] ~~Attached issue to pull request~~
- [x] Changelog entry
- [x] Type annotations are present
- [x] Code is clear and sufficiently documented
- [x] No (preventable) type errors (check using make mypy or make mypy-diff)
- [x] Sufficient test cases (reproduces the bug/tests the requested feature)
- [x] Correct, in line with design
- [x] ~~End user documentation is included or an issue is created for end-user documentation (add ref to issue here: )~~
- [x] ~~If this PR fixes a race condition in the test suite, also push the fix to the relevant stable branche(s) (see [test-fixes](https://internal.inmanta.com/development/core/tasks/build-master.html#test-fixes) for more info)~~
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-tool-ready This ticket is ready to be merged in

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants