Skip to content
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.

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

Fix parsing of Keycloak-related client configuration options #207

Merged
merged 1 commit into from
Jul 10, 2024

Conversation

kerberizer
Copy link
Contributor

This pull request addresses two issues.

Issue 1: Keycloak Configuration Parsing

The pull request enables parsing of the client configuration when Keycloak-related options are present. Specifically, it now supports the following options:

  • hs_keycloak_client_id
  • hs_keycloak_realm
  • hs_keycloak_uri

Previously, client commands would fail with a confusing error message like the following:

Traceback (most recent call last):
  File "(...)/bin/hsls", line 5, in <module>
    from h5pyd._apps.hsls import main
  File "(...)/site-packages/h5pyd/_apps/hsls.py", line 17, in <module>
    cfg = Config()
          ^^^^^^^^
  File "(...)/site-packages/h5pyd/_apps/config.py", line 130, in __init__
    raise ValueError(f"undefined option: {name}")
ValueError: undefined option: ignore

Issue 2: Correcting ValueError Parameter

The second issue is related to the ValueError exception above, which was using the incorrect parameter. The name variable was mistakenly used, leading to confusion, as it was actually a list of known options generated earlier in the code (see this line, where it gets last set to ignore, the last option in default_cfg). The pull request corrects this by using the k variable, which holds the current option name being parsed (v holds the value).

If additional subkeys (e.g., flags) need to be added to the new default_cfg keys, please advise on where to find the necessary information.

@jreadey
Copy link
Member

jreadey commented Jul 8, 2024

Thanks for the PR!

For the default_cfg, could you add the :"flags" line for the new configs? E.g. "flags": ["--keycloak-client-id",]
This just enables options to be set on the command line superseding any environment variable setting.

@kerberizer
Copy link
Contributor Author

@jreadey Thank you for having a look at this!

I've now added the "flags" keys for the new configurations as suggested (I hadn't realized there were no other flag-specific configurations involved -- thank you for the explanation).

@jreadey jreadey merged commit 76ea3de into HDFGroup:master Jul 10, 2024
10 checks passed
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