-
Notifications
You must be signed in to change notification settings - Fork 345
Ingest --tags, --response-headers from CLI options #497
base: master
Are you sure you want to change the base?
Conversation
b641cb7
to
c178911
Compare
@sampsonj LGTM @abstractj Perhaps we should disable |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@slaskawi @sampsonj sorry for the very late feedback, the PR fixes the issues with --tags
which is great and thanks for that @sampsonj. But it also introduces the injection of --response-headers
, something out of the scope for https://issues.redhat.com/browse/KEYCLOAK-12433.
Do we have any use case on why we should introduce the injection of --response-headers
? I could confirm that the issue with tags
is gone, but trying to inject --response-headers
fails, at least for me. That means that we might be fixing the bug for tags and introducing a new bug with response-headers
.
Maybe I'm wrong, but I'd rather to have anything that's out of scope for the injection of attributes on "Custom pages" discussed on the dev mailing list.
@slaskawi regards |
Thanks for having a look @slaskawi and @abstractj - much appreciated! The reason for the increase in scope from the original JIRA ticket is due to the general nature of the unit test - it finds all map based properties as defined in the config and ensures they can be ingested from CLI flags. In writing that test, it identified response headers not being ingested and in order for that test to pass, I needed to add the As for the Thanks for your time - let me know if I've missed something here and I'd be happy to push an update. |
This PR addresses KEYCLOAK-12433, the
--flags
CLI option would not be parsed.See JIRA issue for further details.
Changes include:
--flags
and--response-headers
CLI options to be ingested=
character)