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

Reading hardware data from stdin #92

Merged
merged 3 commits into from
May 6, 2020
Merged

Reading hardware data from stdin #92

merged 3 commits into from
May 6, 2020

Conversation

gauravgahlot
Copy link
Contributor

@gauravgahlot gauravgahlot commented May 4, 2020

At present, the hardware data is pushed as shown below and is not the right (or best) way to do it:

$ tink hardware push 'hadware-data-json'

With the code changes in place, a user can now pipe the hardware data from a JSON file. For example, if the data is saved in /tmp/data.json, you can push the data with either of the following:

$ cat /tmp/data.json | tink hardware push
$ tink hardware push --file /tmp/data.json

Also, if there is no error a confirmation message saying Hardware data pushed successfully is received after the data is pushed.

Signed-off-by: Gaurav Gahlot gaurav.gahlot19@gmail.com

@gauravgahlot gauravgahlot added the kind/feature Categorizes issue or PR as related to a new feature. label May 4, 2020
@gauravgahlot gauravgahlot self-assigned this May 4, 2020
@gauravgahlot gauravgahlot changed the title Reading hardware data from Stdin Reading hardware data from stdin May 4, 2020
@gauravgahlot gauravgahlot linked an issue May 4, 2020 that may be closed by this pull request
mikemrm
mikemrm previously requested changes May 4, 2020
Copy link

@mikemrm mikemrm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me, however we need to update the documentation and examples since this is a breaking change.

@nathangoulding
Copy link
Contributor

nathangoulding commented May 4, 2020

I think having the capability to read the hardware data from a file is definitely advantageous, especially for large blobs and/or a process that writes to a hardware file asynchronously.

My suggestion is that we add the stdin as you've done it, but also have a -f <file> parameter to support the existing file-based mechanism.

Copy link
Contributor

@nathangoulding nathangoulding left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comment.

@gauravgahlot
Copy link
Contributor Author

gauravgahlot commented May 4, 2020

@nathangoulding That makes sense, I will do the changes.
@mikemrm I didn't update the docs here because I'm of the opinion that tinkerbell.org should serve as the sole documentation resource. And, so we can remove all the duplicate documentation from Tink.

Please let me know your thoughts on this.

Signed-off-by: Gaurav Gahlot <gaurav.gahlot19@gmail.com>
Signed-off-by: Gaurav Gahlot <gaurav.gahlot19@gmail.com>
Signed-off-by: Gaurav Gahlot <gaurav.gahlot19@gmail.com>
@gauravgahlot gauravgahlot removed the request for review from mikemrm May 6, 2020 20:19
@nathangoulding nathangoulding dismissed mikemrm’s stale review May 6, 2020 20:35

will update the docs as part of a different PR

@nathangoulding nathangoulding merged commit 01f2e19 into tinkerbell:master May 6, 2020
@gauravgahlot gauravgahlot deleted the read-hw-data branch May 6, 2020 20:36
@alexellis
Copy link
Contributor

This looks great guys. Nice work on it. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

tink hardware push returns no useful data
4 participants