Skip to content

Conversation

@ogenstad
Copy link
Contributor

@ogenstad ogenstad commented Mar 7, 2025

Adds support for adding a request context to the SDK. This can be done at the root level of the client object or on individual requests. The idea here is that when running a generator we'd want to simplify this process for end users so that we add the context to the cloned SDK instance when running within the pipeline where as we wouldn't use this option when using infrahubctl.

Within the Infrahub pipeline we'd still need to be able to specify that specific objects should be saved using a context, such as when creating a validator or a check, then we are not able to set the context for the entire SDK instance.

There are still some calls missing (such as when creating branches etc) and tests would need to be written.

It would be used either like this:

from infrahub_sdk import InfrahubClient
from infrahub_sdk.context import RequestContext

context = RequestContext(account={"id": "182a86a2-05f0-37f8-58df-1746c80a346e"})

client = InfrahubClient()
client.request_context = context

data = {
    "name": "blue-tag",
}
obj = await client.create(kind="BuiltinTag", data=data)
await obj.save()

Alternatively like this:

from infrahub_sdk import InfrahubClient
from infrahub_sdk.context import RequestContext

context = RequestContext(account={"id": "182a86a2-05f0-37f8-58df-1746c80a346e"})

client = InfrahubClient()

data = {
    "name": "blue-tag",
}
obj = await client.create(kind="BuiltinTag", data=data)
await obj.save(request_context=context)

@codecov
Copy link

codecov bot commented Mar 7, 2025

Codecov Report

Attention: Patch coverage is 47.45763% with 31 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
infrahub_sdk/node.py 56.75% 11 Missing and 5 partials ⚠️
infrahub_sdk/protocols_base.py 0.00% 0 Missing and 8 partials ⚠️
infrahub_sdk/context.py 0.00% 6 Missing ⚠️
infrahub_sdk/generator.py 0.00% 1 Missing ⚠️
@@                 Coverage Diff                  @@
##           infrahub-develop     #296      +/-   ##
====================================================
- Coverage             70.62%   70.56%   -0.07%     
====================================================
  Files                    85       86       +1     
  Lines                  7824     7867      +43     
  Branches               1513     1523      +10     
====================================================
+ Hits                   5526     5551      +25     
- Misses                 1908     1920      +12     
- Partials                390      396       +6     
Flag Coverage Δ
integration-tests 21.77% <16.94%> (+0.09%) ⬆️
python-3.10 45.16% <22.03%> (-0.13%) ⬇️
python-3.11 45.16% <22.03%> (-0.15%) ⬇️
python-3.12 45.13% <22.03%> (-0.15%) ⬇️
python-3.13 45.13% <22.03%> (-0.18%) ⬇️
python-3.9 44.03% <22.03%> (-0.16%) ⬇️
python-filler-3.12 24.53% <23.72%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
infrahub_sdk/client.py 67.56% <100.00%> (+0.17%) ⬆️
infrahub_sdk/generator.py 35.44% <0.00%> (-0.46%) ⬇️
infrahub_sdk/context.py 0.00% <0.00%> (ø)
infrahub_sdk/protocols_base.py 79.80% <0.00%> (ø)
infrahub_sdk/node.py 77.19% <56.75%> (-0.40%) ⬇️

... and 3 files with indirect coverage changes

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ogenstad ogenstad force-pushed the pog-request-context-IFC-1340 branch from 2a4e4ec to df19431 Compare March 7, 2025 14:53
Copy link
Contributor

@dgarros dgarros left a comment

Choose a reason for hiding this comment

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

With this implementation, will it be possible to define the context globally when we initialize the client ? I think this is important to support this option in addition to specifying a specific context for a specific request

@ogenstad
Copy link
Contributor Author

ogenstad commented Mar 8, 2025

With this implementation, will it be possible to define the context globally when we initialize the client ? I think this is important to support this option in addition to specifying a specific context for a specific request

If you check the first example in the description that's very close to this. The scenario I was picturing was specifically for use with the generators, where we'd want to set the context on a cloned client after it was cloned. The use case I'd see to have it directly on the initialize would be when we support running jobs within an isolated Docker container or pod, but perhaps you have some other use case that I'm missing?

Copy link
Contributor

@dgarros dgarros left a comment

Choose a reason for hiding this comment

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

If you check the first example in the description that's very close to this. The scenario I was picturing was specifically for use with the generators, where we'd want to set the context on a cloned client after it was cloned. The use case I'd see to have it directly on the initialize would be when we support running jobs within an isolated Docker container or pod, but perhaps you have some other use case that I'm missing?

My bad I missed it, this is exactly what I had in mind too

@ogenstad ogenstad force-pushed the pog-request-context-IFC-1340 branch from df19431 to 03772f1 Compare March 10, 2025 14:03
@ogenstad
Copy link
Contributor Author

ogenstad commented Mar 10, 2025

I've also updated the protocols that were falling behind, will revisit with an integration test once we have an image to test against. This will also be used in #5985.

@ogenstad ogenstad merged commit 2ea79fe into infrahub-develop Mar 11, 2025
17 checks passed
@ogenstad ogenstad deleted the pog-request-context-IFC-1340 branch March 11, 2025 07:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants