Add option to specify Subject

[embano1]

Could not find any related issue/discussion. It would be nice to set a ce-subject in build/send commands. If you think that's useful I can create a PR.

[cardil]

Makes sense, although Subject is considered optional. I'm thinking that maybe a general way of setting optional Cloud Event fields be better?!? Something similar to --field arg, like:

kn event build \
  --ce-field subject=Acme \
  --field email=a@b.c

WDYT?

[embano1]

Makes sense, although Subject is considered optional. I'm thinking that maybe a general way of setting optional Cloud Event fields be better?!? Something similar to --field arg, like:

I think that makes a nice API and could later be combined with a --ce-version flag to indicate and validate against a specific CE version (breaking/non-breaking).

So do you think of migrating the existing CE metadata fields as well, i.e. not only optional fields in --ce-field for consistency?

[cardil]

I feel like the non-optional fields should stay as they are, as they are often used. Exposing them would be easier to use and easier for newcomers to get familiar with CE basics - they can just invoke kn event build --help to get help on common options.

[embano1]

I understand your rational but that would ultimately lead to an inconsistent API, especially for newcomers I think.

So I'm in favor of harmonizing them with consistent prefixes, provide defaults (e.g. for required fields), descriptions and examples in the help section.

E.g. one might want to add additional context attributes later, which could be passed with --ce-attribute so everything is consistent for envelop fields.

[cardil]

All those required fields have some defaults already. Calling:

$ docker run --rm -p 8080:8080 quay.io/openshift-knative/knative-eventing-sources-event-display &
$ kn event send --to-url http://localhost:8080
Jun 30 13:56:36.582   INFO Event (ID: c7665942-d711-4036-849e-e87956ab34d4) have been sent.
☁️  cloudevents.Event
Validation: valid
Context Attributes,
  specversion: 1.0
  type: dev.knative.cli.plugin.event.generic
  source: kn-event/v0.3.0-8-g7ac0651
  id: c7665942-d711-4036-849e-e87956ab34d4
  time: 2021-06-30T11:56:36.580565532Z
  datacontenttype: application/json
Data,
  {}

So, the type, source, id and time are all set, by default.

[embano1]

Yes, I know they exist :)

But consider the changes for optional envelop fields, we would end up with different fields for:

• required fields (e.g. id)
• optional fields (e.g. --ce-subject)
• payload fields (--field)

So my thought was keeping --field but harmonizing required/optional envelop fields with --ce- prefix for consistency. We get sorted flags for free and have a clear distinction between CE-related fields and data (payload) fields, e.g.:

--ce-id (required but with default value)
--ce-subject (optional, no default)
--ce-type (required but with default value)
--field (optional, no default)

[embano1]

Closing this as there is no consensus.

[cardil]

Closing this as there is no consensus.

I think it's worth having this ticket open for a while longer, as this is a real issue. We just need to settle on an API, and I think we could get to it shortly.

/cc @rhuss

[rhuss]

tbh, for known fields i'm woudl support typed fields over key=value options for these reasons:

• Better help messages that show the possible options
• Bash/Zsh completion easily possible
• Better validation on the option parsing level
• Easier to memorize
• The usual way how to work on a UI

However, I probably would even omit the ce- prefix as it sounds redundant.

[cardil]

I have the same opinion as @rhuss on this, now. I think we should just add --subject option.

I didn't added it initially because I just missed it, tbh. For me, type and source are well enough. But, subject is part of CE spec so it should be supported here as well.

[github-actions]

This issue is stale because it has been open for 90 days with no
activity. It will automatically close after 30 more days of
inactivity. Reopen the issue with /reopen. Mark the issue as
fresh by adding the comment /remove-lifecycle stale.

[cardil]

/remove-lifecycle stale

[github-actions]

This issue is stale because it has been open for 90 days with no
activity. It will automatically close after 30 more days of
inactivity. Reopen the issue with /reopen. Mark the issue as
fresh by adding the comment /remove-lifecycle stale.

[cardil]

/remove-lifecycle stale

[github-actions]

This issue is stale because it has been open for 90 days with no
activity. It will automatically close after 30 more days of
inactivity. Reopen the issue with /reopen. Mark the issue as
fresh by adding the comment /remove-lifecycle stale.

[cardil]

/reopen
/remove-lifecycle stale
/triage accepted

[knative-prow]

@cardil: Reopened this issue.

In response to this:

/reopen
/remove-lifecycle stale
/triage accepted

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.