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

Support run/plan jobfile via URL location #1511

Merged
merged 9 commits into from
Aug 17, 2016
Merged

Support run/plan jobfile via URL location #1511

merged 9 commits into from
Aug 17, 2016

Conversation

nak3
Copy link
Contributor

@nak3 nak3 commented Aug 3, 2016

No description provided.

@nak3
Copy link
Contributor Author

nak3 commented Aug 3, 2016

Suppot jobfile via URL like below:

$  nomad run https://raw.githubusercontent.com/nak3/nomad-playground/master/jobs/nginx.nomad
==> Monitoring evaluation "77543788"
    Evaluation triggered by job "nginx"
    Allocation "4d5af5c7" created: node "d9525e74", group "cache"
    Evaluation status changed: "pending" -> "complete"
==> Evaluation "77543788" finished with status "complete"

@dadgar
Copy link
Contributor

dadgar commented Aug 4, 2016

This is cool! I have three requests:

  1. Lets use go-getter. This will let us download jobs in a bunch of ways.

  2. Lets support nomad validate

  3. Add documentation to the commands and website explaining that we support downloading anything in go-getter syntax

@nak3
Copy link
Contributor Author

nak3 commented Aug 6, 2016

Thank you. I added 2) support nomad validate.

But please let me discuss about using go-getter.
Current implentation doesn't download the job, but use HTTP response body like this.

While, if we use go-getter with supported protocol (e.g git/hg), it will not download only one jobfile, but git/hg clone entire repository. So, I feel it is not efficient.

(For example...) If I try to use this git project file with go-getter, it will clone entire git project and specify the one file.

@dadgar
Copy link
Contributor

dadgar commented Aug 8, 2016

Hey @nak3,

In the specific case that you give it a git repo, you are correct that it will download the whole thing. However, if you give it the raw url to the file in github like (https://raw.githubusercontent.com/nak3/nomad-playground/master/jobs/nginx.nomad) it will download it using HTTP. By using go-getter you can download the file from more locations which is nice.

We should also refactor the code into a helper so that we don't duplicate everywhere

@nak3
Copy link
Contributor Author

nak3 commented Aug 11, 2016

@dadgar OK, I understand. I update them to use go-getter. Thank you.

@@ -216,3 +222,63 @@ READ:
// Just stream from the underlying reader now
return l.ReadCloser.Read(p)
}

type Helper struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

Lets change the name to JobGetter and add a comment on the struct

@dadgar
Copy link
Contributor

dadgar commented Aug 15, 2016

Small comments but LGTM

@nak3
Copy link
Contributor Author

nak3 commented Aug 16, 2016

Thank you. I updated them.

@dadgar
Copy link
Contributor

dadgar commented Aug 16, 2016

@nak3 Looks like the tests are failing

@nak3
Copy link
Contributor Author

nak3 commented Aug 17, 2016

@dadgar I don't think test was failed due to this PR. I fetched and re-push the PR, and now it passed tests.

@dadgar
Copy link
Contributor

dadgar commented Aug 17, 2016

Ah... The travis tests misreport their success right now. Working on that. Here are the raw logs:
https://s3.amazonaws.com/archive.travis-ci.org/jobs/152858750/log.txt

TestPlanCommand_From_URL
TestRunCommand_Fails
....

@nak3
Copy link
Contributor Author

nak3 commented Aug 17, 2016

Oh, sorry. I missed it. Thank you. I updated.

@dadgar
Copy link
Contributor

dadgar commented Aug 17, 2016

Awesome thank you so much!

@dadgar dadgar merged commit 2b1ccd4 into hashicorp:master Aug 17, 2016
@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 18, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants