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

feat: Add support for using jobspec2 parser #398

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

alexdulin
Copy link

This adds in support for using the HCL2 jobspec2 parser with a new -hcl2 flag.
This allows for using new features like upstreams.datacenter and terminating
gateways. It should also allow for new features in the future to be used that
are only being added to the new jobspec2 parser.

Since using the jobspec2 parser and HCL2 can break existing job files, I decided to implement this as opt-in rather than opt-out, having the default be to use the original jobspec parser.

Signed-off-by: Alex Dulin alex@morningconsult.com

@hashicorp-cla
Copy link

hashicorp-cla commented Mar 8, 2021

CLA assistant check
All committers have signed the CLA.

@alexdulin
Copy link
Author

@jrasell Any chance this could get a review? Its been open for a while now and adds quite a bit of functionality and fixes to other issues people have been opening.

Apologies in advance for @'ing you directly.

Copy link
Member

@jrasell jrasell left a comment

Choose a reason for hiding this comment

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

Thanks @alexdulin and apologies for the delay in reviewing. I mostly have a few questions that I would like your feedback on which are inline.

go.mod Outdated Show resolved Hide resolved
go.mod Outdated Show resolved Hide resolved
template/render.go Show resolved Hide resolved
This adds in support for using the HCL2 jobspec2 parser with a new -hcl2 flag.
This allows for using new features like `upstreams.datacenter` and terminating
gateways. It should also allow for new features in the future to be used that
are only being added to the new jobspec2 parser.

Signed-off-by: Alex Dulin <alex@morningconsult.com>
@alexdulin
Copy link
Author

@jrasell addressed all your comments in initial review. Thanks!

@alexdulin
Copy link
Author

@tgross @jrasell I apologize for bugging you in advance, but is there any chance of this getting merged? All comments were addressed almost half a year ago and it would be great to get off of the fork of Levant we currently use, which is based on this branch.

If this MR is not going anywhere, it would be greatly appreciated to know that so we can start implementing more long term solutions on our own.

@jrasell
Copy link
Member

jrasell commented Nov 15, 2021

@alexdulin i'll put this on my list for early this week!

@bbarman4u
Copy link

Hello team it is 2022 and seems like this important PR is still in the review stage.

@jrasell
Copy link
Member

jrasell commented Feb 16, 2022

@alexdulin and @bbarman4u yep my bad, this slipped through the gaps. I'll try and find some time to give this another look and test.

@droustchev
Copy link

I would be really interested in seeing this PR merged in as well 👍

@mharp-zenimax
Copy link

Any chance this could get merged in?

@cyrilgdn cyrilgdn mentioned this pull request Dec 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants