Enhance git credential helper performances #6372
Conversation
LucasG0
changed the title
github-actions
bot
added
the
group/backend
CodSpeed Performance ReportMerging #6372 will not alter performanceComparing Summary
|
| @@ -51,7 +51,7 @@ def get( | |||
| raise typer.Exit(1) from exc | |||
|
|
|||
| client = InfrahubClientSync(config=Config(address=config.SETTINGS.main.internal_address, insert_tracker=True)) | |||
| repo = client.get(kind=InfrahubKind.GENERICREPOSITORY, location__value=location) | |||
| repo = client.get(kind=CoreGenericRepository.__name__, location__value=location) | |||
There was a problem hiding this comment.
This avoids importing above infrahub.core.constants, even if performance improvement is negligible. One comment on this is that all client sync methods expect dedicated sync protocols as input, but they are actually buggy as we deduce the kind from their class name (eg: CoreGenericRepositorySync in an invalid kind). That is what I used __name__ above to avoid breaking typing while using the non-sync protocol CoreGenericRepository.
There was a problem hiding this comment.
I opened a bug in the SDK for this: opsmill/infrahub-sdk-python#380
Not convinced if we should go with the above or just use the hardcoded string "CoreGenericRepository", for now, as we're sending in a string in the above example it doesn't actually provide any extra value. Aside from having some loose reference to a "constant".
There was a problem hiding this comment.
Aside from having some loose reference to a "constant"
Yes that is the idea, I feel it's still better than a hardcoded string even though it is not ideal..
LucasG0
force-pushed
the
lgu-boost-git-credential-helper
branch
2 times, most recently
from
dccd832 to
ee4bd26
Compare
From
time python helper.py: elapsed 1.31sto
time python helper.py: elapsed 0.26sThe main improvement comes from the fact
git_credential.helperimportsinfrahub.config, itself importinginfrahub.database.constantswhich imports a lot of code asinfrahub.database.__init__contains a lot of logic. This PR moves these database constants in a separatedinfrahub.constantspackage that is fast to import. Note the long term fix would be to cleaninfrahub.database.__init__in order to finally restore these constants withininfrahub.database.