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

Add parameter "namespaces" #522

Merged
merged 4 commits into from
Mar 19, 2024
Merged

Add parameter "namespaces" #522

merged 4 commits into from
Mar 19, 2024

Conversation

dr0i
Copy link
Member

@dr0i dr0i commented Mar 18, 2024

See #505.

Set a namespace or a list of namespaces. A namespace is a key-value structure, separated by a space. Multiple namespaces are separated by a semicolon.

@dr0i dr0i requested a review from blackwinter March 18, 2024 14:01
Copy link
Member

@blackwinter blackwinter left a comment

Choose a reason for hiding this comment

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

Are you sure that the semicolon is suitable as namespace separator? Then you can't set a namespace name containing this character (see also similar discussion regarding multiple headers in #456).

Also, the space as key-value separator seems a little unusual. I would suggest we either adopt the full syntax of Java property files (which the namespace file setter uses) or model this new setter after the header setter in HttpOpener (I'm not sure if there are other precedents for this kind of functionality).

Finally, using an empty key (prefix) for the default namespace feels a little awkward. I suspect that this is a consequence of the previous behaviour, but I would suggest that we either make the key optional in this case or add support for __default as the key for the default namespace (this is also the default map key for Metamorph Maps).

@dr0i dr0i marked this pull request as draft March 19, 2024 12:53
@dr0i dr0i force-pushed the 505-setNamespacesFromFlux branch 2 times, most recently from 95b618c to dc1f6c8 Compare March 19, 2024 13:01
@dr0i
Copy link
Member Author

dr0i commented Mar 19, 2024

I've changed to your proposals. Will squash when you approve and add a PG example.

@dr0i dr0i assigned blackwinter and unassigned dr0i Mar 19, 2024
Copy link
Member

@blackwinter blackwinter left a comment

Choose a reason for hiding this comment

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

I think the syntax description could be more accurate, but overall 👍

@blackwinter blackwinter assigned dr0i and unassigned blackwinter Mar 19, 2024
dr0i and others added 2 commits March 19, 2024 14:33
Set a namespace or a list of namespaces. A namespace is a Java Properties
structure, i.e. a key-value structure where the key is separated from the value
by an equal sign '=', a semicolon ':' or white space ' '.Multiple namespaces
are separated by a line feed '\n'.
@dr0i dr0i force-pushed the 505-setNamespacesFromFlux branch from a2a43ad to 874505f Compare March 19, 2024 13:36
@dr0i dr0i merged commit c42285a into master Mar 19, 2024
1 check passed
@dr0i dr0i deleted the 505-setNamespacesFromFlux branch March 19, 2024 14:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants