Skip to content

Add support for request context #296

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
Merged
merged 1 commit into from
Mar 11, 2025
Merged

Add support for request context #296

merged 1 commit into from
Mar 11, 2025

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 •
edited
Loading

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
dgarros
dgarros reviewed Mar 7, 2025
View reviewed changes
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?

dgarros
dgarros approved these changes Mar 9, 2025
View reviewed changes
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 •
edited
Loading

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 mentioned this pull request Mar 10, 2025
Merged
@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

Reviewers

@dgarros dgarros dgarros approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ogenstad @dgarros