awx icon indicating copy to clipboard operation
awx copied to clipboard

Move plugin loading to dispatcher startup

Open AlanCoding opened this issue 11 months ago • 3 comments

SUMMARY

If you read some of the comments of the methods moved here, it was very clear even when implemented that you're not supposed to be interacting with the database in the .ready method.

To summarize some discussion:

  • the dispatcher startup is the "middle-ground" solution, not too often, not too infrequently
    • the migration hook would be much more infrequent, but awx-operator does not reliably call it every startup
    • the ready method calls every time ever service or any other script using Django starts
  • The remaining concern is that you could have a race condition, expecting a CredentialType for configuration-as-code, which is technically possible
    • This is not actually realistic in my opinion
    • If there is a problem, scripts can poll until the capacity of instances reaches a non-zero value, which happens after these methods in the startup method
  • I am very opinionated to move away from standard Django practices, as I have another patch up at https://github.com/ansible/awx/pull/15463 to remove another case where another query-on-import was performed. Why?
    • Log maintenance becomes a nightmare because these categorically front-run any other database errors, which I spend a lot of time debugging
    • People will find it more and more difficult to work with AWX because it doesn't use Django out-of-the-box. If there were a really strong reason to break standard practice, I won't resist, we may have to do. This does not appear to be a strong reason.
ISSUE TYPE
  • Bug, Docs Fix or other nominal change
COMPONENT NAME
  • API

AlanCoding avatar Jan 10 '25 16:01 AlanCoding

Running the development environment seems to go fine with this. Additionally, making sure methods are well-behaved:

In [1]: from awx.main.tasks.system import *

In [2]: load_inventory_plugins()

In [3]: load_credential_types_feature()

The 2nd one takes longer (maybe 0.1 second) but not worried in any case.

AlanCoding avatar Jan 10 '25 16:01 AlanCoding

Quality Gate Failed Quality Gate failed

Failed conditions
B Maintainability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

sonarqubecloud[bot] avatar Jan 10 '25 18:01 sonarqubecloud[bot]

Leaning towards closing this, as talking with @elyezer we would prefer consolidation management commands and doing this in one of those.

AlanCoding avatar Sep 24 '25 13:09 AlanCoding