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

Enable ignoring namespace guard for handle-marcxml in flux #569

Closed
TobiasNx opened this issue Nov 12, 2024 · 6 comments · Fixed by #575
Closed

Enable ignoring namespace guard for handle-marcxml in flux #569

TobiasNx opened this issue Nov 12, 2024 · 6 comments · Fixed by #575
Assignees

Comments

@TobiasNx
Copy link
Contributor

TobiasNx commented Nov 12, 2024

@blackwinter introduced the possibility to turn off the namespace guard with handle-marcxml but at the moment it only works in Java due to setting the option namespace=null. Assining null is not possible in FLUX.

So you mean that you can't use namespace="" with both records with and without
namespace? That's where namespace=null comes in, but Flux does not support
specifying null values.

Correct. We need to decide how to support this. Should I open a ticket to support this? Or is it a better idea to introduce an boolean-option that turns this test off?

Originally posted by @TobiasNx in #330 (comment)

hbz/Verbundgruppe has the usecase to use the same workflow for transformation marcxml WITH and WITHOUT namespace. This would require this enhancement.

@dr0i
Copy link
Member

dr0i commented Nov 21, 2024

Value "null" is no valid URI anyway, and from user perspective it's easier to simply always assigning anything as a String ("").
@blackwinter please explain why we don't want to just allow namespace="null" to behave like setNamespace(null).

@dr0i dr0i assigned blackwinter and unassigned dr0i Nov 21, 2024
@dr0i dr0i moved this from Selected to Review in Metafacture Nov 21, 2024
@blackwinter
Copy link
Member

Are you saying that we should derive meaning from an opaque string just because of limitations in the grammar specification? Are you also saying that it should only apply to the MarcXmlHandler in this specific instance, not to Flux in general?

In that case, I would very much prefer to introduce another option to disable the namespace check (boolean). I probably shouldn't have done it this way to begin with.

@blackwinter blackwinter removed their assignment Nov 21, 2024
@blackwinter blackwinter moved this from Review to Backlog in Metafacture Nov 21, 2024
@dr0i
Copy link
Member

dr0i commented Nov 21, 2024

... limitations in the grammar specification?

At this point I see them as guardrails.
Just setting something to null is not self-explaining. So we would have to document what the semantics of null would be, anyway.

In that case, I would very much prefer to introduce another option to disable the namespace check (boolean).

I vote for this.

I probably shouldn't have done it this way to begin with.

As your implementation is useful only to programmers I think they will cope with it. But of course it would have been the best to implement something that can be also used by flux.

@blackwinter
Copy link
Member

I'm sorry, but I don't follow this line of argument. You would still need to document what "null" means in this context. I, for one, really think that Flux should support other (primitive) data types as well; I've been bothered by "true" or "42" whenever I had to write it just to satisfy the grammar. You lose out on expressiveness. Then again, I don't use Flux that often, so I can't really complain. If you think that the limitation to strings makes it easier in teaching and support, I'll accept your word for it.

dr0i added a commit that referenced this issue Nov 21, 2024
Setting "ignorenamespace" to "true" ignores checking the namespace.
dr0i added a commit that referenced this issue Dec 2, 2024
Even if a namespace is set after setting "ignorenamespace" to "true" the
namespace is ignored.
dr0i added a commit that referenced this issue Dec 2, 2024
@dr0i dr0i self-assigned this Dec 2, 2024
@dr0i dr0i moved this from Backlog to Review in Metafacture Dec 2, 2024
@dr0i dr0i closed this as completed in #575 Dec 2, 2024
@github-project-automation github-project-automation bot moved this from Review to Done in Metafacture Dec 2, 2024
@TobiasNx
Copy link
Contributor Author

TobiasNx commented Dec 2, 2024

Should I test this feature?

@dr0i
Copy link
Member

dr0i commented Dec 2, 2024

Yeah - you can give it a test. I thought this functionality too simple to need a functional review but it doesn't hurt of course.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants