-
Notifications
You must be signed in to change notification settings - Fork 231
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
[#903]: Make schema optional, allow alternate content type #905
[#903]: Make schema optional, allow alternate content type #905
Conversation
f149892
to
a1e6e0b
Compare
Hm, looks like you have a CI check in place that doesn't allow to properly specify the copyright year range. |
Yes, copyright header for us is only the creation year as this is sufficient according to the Eclipse Foundation Handbook. |
Works for me 👍 |
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.
Thx @ctron, already looks very promising :)
Added a few requests..
One more thing:
Could you please enhance the services/gateway/starter/src/main/resources/gateway.conf
with the cloud-events
section (e.g. below public-health { ... }
) and the same defaults as in the code, so:
cloud-events {
empty-schema-allowed = true
data-types = [
"application/json"
"application/vnd.eclipse.ditto+json"
]
}
That way Ditto's configuration files act as documentation of what can be configured overall.
...n/java/org/eclipse/ditto/services/gateway/endpoints/routes/cloudevents/CloudEventsRoute.java
Outdated
Show resolved
Hide resolved
.../java/org/eclipse/ditto/services/gateway/util/config/endpoints/DefaultCloudEventsConfig.java
Outdated
Show resolved
Hide resolved
.../java/org/eclipse/ditto/services/gateway/util/config/endpoints/DefaultCloudEventsConfig.java
Outdated
Show resolved
Hide resolved
.../java/org/eclipse/ditto/services/gateway/util/config/endpoints/DefaultCloudEventsConfig.java
Outdated
Show resolved
Hide resolved
...rc/main/java/org/eclipse/ditto/services/gateway/util/config/endpoints/CloudEventsConfig.java
Outdated
Show resolved
Hide resolved
.../java/org/eclipse/ditto/services/gateway/util/config/endpoints/DefaultCloudEventsConfig.java
Outdated
Show resolved
Hide resolved
a1e6e0b
to
34c3d67
Compare
I just updated the PR. |
...n/java/org/eclipse/ditto/services/gateway/endpoints/routes/cloudevents/CloudEventsRoute.java
Outdated
Show resolved
Hide resolved
...st/java/org/eclipse/ditto/services/gateway/endpoints/routes/cloudevents/CloudEventsTest.java
Outdated
Show resolved
Hide resolved
...st/java/org/eclipse/ditto/services/gateway/endpoints/routes/cloudevents/CloudEventsTest.java
Outdated
Show resolved
Hide resolved
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.
The CloudEventsTests
would need meaningful tests, but I'm not sure if you want to address all mentioned follow-up tasks of #903 ;)
I would rather delete the current CloudEventsTests
class and the one not so useful test to be honest.
34c3d67
to
1b05c63
Compare
I wanted to keep the scope of this PR as is, if possible. And work on the other items in a new PR. Just to keep PRs small. I tried to explain the idea behind the current state of the unit test above. But as mentioned, I am open to dropping it altogether for this PR. |
1b05c63
to
fe7d03d
Compare
This change allows to make the schema optional and also allows to configure the data content type. Signed-off-by: Jens Reimann <[email protected]>
fe7d03d
to
ffcfaf2
Compare
Thx for the contribution, really appreciated 👍 |
This change allows to make the schema optional and also allows to
configure the data content type.
Signed-off-by: Jens Reimann [email protected]