Skip to content

Conversation

@FragmentedPacket
Copy link
Contributor

@FragmentedPacket FragmentedPacket commented Jun 14, 2025

Fixes #445

  • Iterate over parameters defined in a check_definition and set the parameters into the GraphQL query properly
  1. This code is still somewhat rudimentary and I think we may want to iterate over parameters and change identifier to a dictionary with the to be properly passed into the GraphQL to support multiple parameters.

…n't change and potential issues arise if the identifier is only valid on the specific object and not the 'CoreGroup'.
@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Jun 14, 2025

Deploying infrahub-sdk-python with  Cloudflare Pages  Cloudflare Pages

Latest commit: 1159059
Status: ✅  Deploy successful!
Preview URL: https://9c426057.infrahub-sdk-python.pages.dev
Branch Preview URL: https://may-202506-445-check-attribu.infrahub-sdk-python.pages.dev

View logs

@codecov
Copy link

codecov bot commented Jun 14, 2025

Codecov Report

❌ Patch coverage is 0% with 7 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
infrahub_sdk/ctl/check.py 0.00% 7 Missing ⚠️
@@            Coverage Diff             @@
##           stable     #446      +/-   ##
==========================================
+ Coverage   75.57%   75.59%   +0.01%     
==========================================
  Files         100      100              
  Lines        8750     8748       -2     
  Branches     1704     1703       -1     
==========================================
  Hits         6613     6613              
+ Misses       1661     1659       -2     
  Partials      476      476              
Flag Coverage Δ
integration-tests 34.64% <0.00%> (+<0.01%) ⬆️
python-3.10 47.86% <0.00%> (-0.02%) ⬇️
python-3.11 47.88% <0.00%> (+0.01%) ⬆️
python-3.12 47.83% <0.00%> (-0.02%) ⬇️
python-3.13 47.83% <0.00%> (-0.02%) ⬇️
python-3.9 46.55% <0.00%> (+0.01%) ⬆️
python-filler-3.12 25.28% <0.00%> (+<0.01%) ⬆️

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

Files with missing lines Coverage Δ
infrahub_sdk/ctl/check.py 27.50% <0.00%> (+0.45%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@FragmentedPacket FragmentedPacket requested a review from a team August 8, 2025 13:48
# TODO: Does this support multiple parameters and we need make identifiers a dict to iterate over on L166-168?
for param_key, param_value in check_module.definition.parameters.items():
identifier = param_key
identifier_attribute = param_value.split("__")[0]
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this might be fine as it resolves a previous error. But I'm not sure these params really work today if you have more than one. This is something that we should address. It's also problematic with regards to a query that might look like this instead:

query TagsQuery($tag_name: String!) {
  BuiltinTag(name__value: $tag_name) {
    edges {
      node {
        description {
          value
        }
      }
    }
  }
}

I.e. it's a valid check but the name is missing and as such will be reported as null.

Have you tried to run this same check and code within the pipeline in Infrahub as well? I think we might need to fix something there too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey @ogenstad, I ran the same check from the issue in the pipeline and it succeeds with no issue.

If I update the GraphQL query to above, the check fails but that is due to the code within the check.

@FragmentedPacket
Copy link
Contributor Author

Closing due to a larger effort required: opsmill/infrahub#7188

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.

bug: Check attempts to use the parameter key, but runs into AttributeError if not a valid attribute.

3 participants