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

Read CE from stdin, or file given by --from option #30

Open
rhuss opened this issue Jun 15, 2021 · 24 comments
Open

Read CE from stdin, or file given by --from option #30

rhuss opened this issue Jun 15, 2021 · 24 comments
Labels
triage/accepted Issues which should be fixed (post-triage)

Comments

@rhuss
Copy link
Contributor

rhuss commented Jun 15, 2021

Summary

Introduce the --from option for build and send commands, which a user can use to load CE from a file. The option defaults to -, which means the STDIN. Introduce the --from-format option as well, which defaults to json.

Description

The send and build commands may read the STDIN by default and expect there an event in JSON format.

This creates a nice and clean Unix-friendly flow:

$ kn event build --field a.b.c=foo | kn event send --to event-display

User might change that behavior with a new --from option:

$ kn event build --field a.b.c=foo -o json > event.json
# and later
$ kn event send --from event.json --to event-display

By adding the new --from option to the build sub-command as well, we could remove the event building options from send sub-command and keep nice and easy flow even for complex things (also with detecting tty on stdin/stdout we can automatically use json format instead of human-readable format):

$ kn event build --field a.b.c=foo
  | some-ce-modifing-command
  | kn event build --field a.d=bar
  | kn event send --to pod:event-sender

(The above example will build a basic CE with a.b.c=foo data, pass it to some command in JSON format, then add a.d=bar fields, and finally send it to K8s pod called event-sender)

Original description by @rhuss:

For users who don't know the CE spec very well, it feels strange to specify the payload of an event with --field event.data=something (also too long). Also we should allow to pick up the date from a file (which is not needed for other fields).

So from a UX pov I propose to introduce something like a --data and --data-from option.

@cardil
Copy link
Contributor

cardil commented Jun 15, 2021

The --field option is to easily build a data for event. It doesn't require any knowledge of CE spec. Using it like:

--field user.email=jdoe@example.org
--field user.name='John Doe'
--field score=154002

will produce a CE with data of:

{
  "user": {
    "email": "jdoe@example.org",
    "name": "John Doe",
  },
  "score": 154002
}

By the way. I also think the --data-from would be a good extension. I did plan to have it. That's also the one of the reason why kn event build command existed.

One could create an event template using kn event build and save it in project resources. And then use kn event send with --data-from (or rather maybe --load-from), and specify some overrides. That would make a nice UX by making command shorter.

Maybe I didn't get what you'd like to see here. Could you rephrase, so I can understand better?

@embano1
Copy link

embano1 commented Jun 27, 2021

Accepting stdin would be neat. Currently I pipe build output to curl which does support -d@- :)

@github-actions
Copy link

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.

@github-actions github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Sep 26, 2021
@rhuss
Copy link
Contributor Author

rhuss commented Sep 27, 2021

/remove-lifecycle stale

@knative-prow-robot knative-prow-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Sep 27, 2021
@github-actions
Copy link

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.

@github-actions github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Dec 27, 2021
@rhuss
Copy link
Contributor Author

rhuss commented Jan 3, 2022

/remove-lifecycle stale

@knative-prow-robot knative-prow-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jan 3, 2022
@github-actions
Copy link

github-actions bot commented Apr 4, 2022

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.

@github-actions github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Apr 4, 2022
@cardil
Copy link
Contributor

cardil commented Apr 26, 2022

At @rhuss:

The issue description is misleading to me. I think you've been mistaken on how --field options works. Do my explanations clarify it for you?

Still, I think the option to provide data from STDIN is a good idea. But, that would rather be a different issue. Or just reword this one.

@github-actions github-actions bot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Apr 27, 2022
@cardil cardil added the triage/needs-user-input Issues which are waiting on a response from the reporter label Jul 19, 2022
@rhuss
Copy link
Contributor Author

rhuss commented Jul 29, 2022

Yeah, maybe I misunderstood the --field option. So, its only for building up the data: payload of a CE, right ? It's not for specifying fields on the CE itself ? ("field" in this sense sounds a bit counterintuitive).

Can you specify the full data directly with a --data, too ? No every CE has a JSON payload, and not sure if --field is capable of adding arrays.

@cardil
Copy link
Contributor

cardil commented Oct 7, 2022

Yes, the --field is only for building the payload in easy way. The idea was that you could build even complex payloads by specifying one value at a time.

For setting custom CE fields, I thought we could add --ce-property option.

The design docs mention --input as a way of loading previously saved CE. I see, that sometimes a better would be an option to load only payload from file. How about --payload option also? So, we would have --input to load the whole Ce as a base, and --payload to load just the payload of CE.

WDYT @rhuss?

@github-actions
Copy link

github-actions bot commented Jan 6, 2023

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.

@github-actions github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jan 6, 2023
@cardil
Copy link
Contributor

cardil commented Jan 30, 2023

/remove-lifecycle stale

ping @rhuss

@knative-prow knative-prow bot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jan 30, 2023
@rhuss
Copy link
Contributor Author

rhuss commented Feb 13, 2023

So, why not just --data for being able to specify the full payload, as this reflects also the CE schema? And I like the suggestion from @embano1 to be able to take the payload only from standard input, too. Like with --data - or --data @-.

@cardil
Copy link
Contributor

cardil commented Mar 23, 2023

The vision doc has a templating section, which boils down to the --input param. The --input can take a file, stdin (-), or http(s) address, and that will expect the full payload (whole CE).

I think the input is a slightly better name than data, as data might imply a meaning it's about the CE data, instead of the whole CE.

@rhuss
Copy link
Contributor Author

rhuss commented May 6, 2023

I would love to see an option to import the CE data only, not the full CE. This is especially useful for complex JSON payload. So I still would like a --data option for the payload only.

@github-actions
Copy link

github-actions bot commented Aug 5, 2023

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.

@github-actions github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Aug 5, 2023
@rhuss
Copy link
Contributor Author

rhuss commented Aug 5, 2023

@cardil is --data something that you would still consider ?

@github-actions github-actions bot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Aug 6, 2023
Copy link

github-actions bot commented Nov 5, 2023

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.

@github-actions github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Nov 5, 2023
@rhuss
Copy link
Contributor Author

rhuss commented Nov 5, 2023

/remove-lifecycle stale

@knative-prow knative-prow bot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Nov 5, 2023
Copy link

github-actions bot commented Feb 4, 2024

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.

@github-actions github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Feb 4, 2024
@rhuss
Copy link
Contributor Author

rhuss commented Feb 6, 2024

/remove-lifecycle stale

@knative-prow knative-prow bot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Feb 6, 2024
Copy link

github-actions bot commented May 7, 2024

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.

@github-actions github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label May 7, 2024
@rhuss
Copy link
Contributor Author

rhuss commented May 8, 2024

/remove-lifecycle stale

@knative-prow knative-prow bot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label May 8, 2024
@cardil
Copy link
Contributor

cardil commented Jun 6, 2024

Yes. I want to do that. In fact, the send command may read the stdin by default and expect there an event in JSON format.

This creates a nice and clean Unix-friendly flow:

$ kn event build --field a.b.c=foo | kn event send --to event-display

User might change that behavior with a new --from option:

$ kn event build --field a.b.c=foo -o json > event.json
# and later
$ kn event send --from event.json --to event-display

By adding the new --from option to the build sub-command as well, we could remove the event building options from send sub-command and keep nice and easy flow even for complex things (also with detecting tty on stdin/stdout we can automatically use json format instead of human-readable format):

$ kn event build --field a.b.c=foo
  | some-ce-modifing-command
  | kn event build --field a.d=bar
  | kn event send --to pod:event-sender

(The above example will build a basic CE with a.b.c=foo data, pass it to some command in JSON format, then add a.d=bar fields, and finally send it to K8s pod called event-sender)

I'll update the issue title and description accordingly. Thanks, @rhuss, for inspiration!

/triage accepted

@knative-prow knative-prow bot added the triage/accepted Issues which should be fixed (post-triage) label Jun 6, 2024
@cardil cardil removed the triage/needs-user-input Issues which are waiting on a response from the reporter label Jun 6, 2024
@cardil cardil changed the title Expose data option more prominently Read CE from stdin, or file given by --from option Jun 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
triage/accepted Issues which should be fixed (post-triage)
Projects
None yet
Development

No branches or pull requests

4 participants