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

Make fields "required" in strategy-specific config classes #359

Open
CasperWA opened this issue Oct 11, 2023 · 0 comments
Open

Make fields "required" in strategy-specific config classes #359

CasperWA opened this issue Oct 11, 2023 · 0 comments

Comments

@CasperWA
Copy link
Contributor

The type explains that the value can only be "text/csv", which means validation will fail if it is anything else. This is the behavior we want for the CSVStrategy.
The second "text/csv" is the default value that will be set for mediaType should it not be explicitly given.

One could argue that mediaType must ALWAYS be explicitly provided, and hence the default value can be changed to an ellipsis (...) to signify this? I don't mind that change at all. This will reflect what is originally intended for the ResourceConfig as well, but can only be realized through a custom validator due to it only being valid if different pairs of fields are given at the same time (mediaType + downloadUrl OR accessUrl + accessService).

Originally posted by @CasperWA in #330 (comment)

The whole discussion here between @daniel-sintef and myself is a nice one, also to give a bit more context.

Essentially, the ResourceConfig fields mediaType, downloadUrl, accessUrl, and accessService are not explicitly set to be "required" fields in the ResourceConfig data model (instead a validator is used to make them "required").
For strategy-specific config classes, i.e., sub-classes of ResourceConfig, it might be nice to have these fields be explicitly "required", since it can be no other way for that particular strategy.

@CasperWA CasperWA mentioned this issue Oct 11, 2023
8 tasks
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

No branches or pull requests

1 participant