Skip to content

Fix type issue in KeyValutLoader#447

Merged
mathialo merged 1 commit intomasterfrom
fix-typing-issue
Jun 11, 2025
Merged

Fix type issue in KeyValutLoader#447
mathialo merged 1 commit intomasterfrom
fix-typing-issue

Conversation

@mathialo
Copy link
Copy Markdown
Contributor

SecretClient does not accept Nones as the credential. This is a
type issue, that mypy only sometimes catches.

There is no reason the credentials should be a class variable, it's
only ever used inside the _init_client method, so it's fine to move it
to a local variable.

`SecretClient` does not accept `None`s as the `credential`. This is a
type issue, that `mypy` only _sometimes_ catches.

There is no reason the `credentials` should be a class variable, it's
only ever used inside the `_init_client` method, so it's fine to move it
to a local variable.
@mathialo mathialo requested a review from a team as a code owner June 11, 2025 08:30
Copy link
Copy Markdown
Contributor

@Hmnt39 Hmnt39 left a comment

Choose a reason for hiding this comment

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

LGTM

@codecov
Copy link
Copy Markdown

codecov bot commented Jun 11, 2025

Codecov Report

Attention: Patch coverage is 75.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 77.47%. Comparing base (914650e) to head (ed0fe30).
Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
cognite/extractorutils/configtools/loaders.py 75.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master     #447   +/-   ##
=======================================
  Coverage   77.47%   77.47%           
=======================================
  Files          42       42           
  Lines        3511     3511           
=======================================
  Hits         2720     2720           
  Misses        791      791           
Files with missing lines Coverage Δ
cognite/extractorutils/configtools/loaders.py 84.09% <75.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@mathialo mathialo added the waiting-for-risk-review Waiting for a member of the risk review team to take an action label Jun 11, 2025
Copy link
Copy Markdown

@larshagencognite larshagencognite left a comment

Choose a reason for hiding this comment

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

🦄

@larshagencognite larshagencognite added risk-review-ongoing Risk review is in progress waiting-for-team Waiting for the submitter or reviewer of the PR to take an action and removed waiting-for-risk-review Waiting for a member of the risk review team to take an action labels Jun 11, 2025
@mathialo mathialo merged commit ac184d5 into master Jun 11, 2025
6 checks passed
@mathialo mathialo deleted the fix-typing-issue branch June 11, 2025 12:01
toondaey added a commit that referenced this pull request Jun 11, 2025
`SecretClient` does not accept `None`s as the `credential`. This is a
type issue, that `mypy` only _sometimes_ catches.

There is no reason the `credentials` should be a class variable, it's
only ever used inside the `_init_client` method, so it's fine to move it
to a local variable.
cognite-bulldozer bot added a commit that referenced this pull request Jun 26, 2025
* Update connection config schema

* feat: cast credentials

* fix: use external id

* feat: update method

* test: test casting

* chore: change variable to match

* test: comprehensive test of full config-schema

* Fix type issue in `KeyValutLoader` (#447)

`SecretClient` does not accept `None`s as the `credential`. This is a
type issue, that `mypy` only _sometimes_ catches.

There is no reason the `credentials` should be a class variable, it's
only ever used inside the `_init_client` method, so it's fine to move it
to a local variable.

* feat: scopes config list

* feat: expose connection parameters

* fix: update cognite sdk version

---------

Co-authored-by: cognite-bulldozer[bot] <51074376+cognite-bulldozer[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk-review-ongoing Risk review is in progress waiting-for-team Waiting for the submitter or reviewer of the PR to take an action

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants