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
(PE-28373) Use Bolt 2.x #70
Conversation
lib/ace/transport_app.rb
Outdated
'transport' => 'remote' | ||
} | ||
} | ||
target = [Bolt::Target.from_hash(target_data, inventory)] | ||
rescue ACE::Error => e | ||
request_error = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm wondering if we can just construct Bolt::Results here. https://github.com/puppetlabs/bolt/blob/master/lib/bolt/result.rb#L10 that way we dont have to keep trying to mimic the structure or a proper result.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope. Any error here and there will not yet be a Target. See puppetlabs/bolt#1693 for parity w/bolt-server
target = [Bolt::Target.new(body['target']['host'] || body['target']['name'], opts)] | ||
inventory = Bolt::Inventory.empty | ||
target_data = { | ||
'name' => body['target']['host'] || body['target']['name'] || 'remote', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems like we used to be able to get away with a target with no name???? Not sure what the impact of this will be...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is in my mind the biggest outstanding question.
Based on #69 for now. Will clean that up once thats merged. update - done. |
Codecov Report
@@ Coverage Diff @@
## master #70 +/- ##
===========================================
- Coverage 100.00% 99.45% -0.55%
===========================================
Files 9 9
Lines 369 369
===========================================
- Hits 369 367 -2
- Misses 0 2 +2
Continue to review full report at Codecov.
|
3702587
to
52043d2
Compare
"object": nil, | ||
"status": "failure", | ||
"result": { | ||
"_error": ACE::Error.to_h(e.message, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lol, not exacly sure what happened here before? But fairly certain that should be .new
6ea2da8
to
ab4d46f
Compare
Make ACE compatible with Bolt 2.x by updating how we make instantiate inventory/targets and return Results. Specifically every task result should at least have the keys `status` and `value`.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
This commit adds back the acceptance directory removed in puppetlabs#70 as our CI expects there to be such directory.
Make ACE compatible with Bolt 2.x by updating how we make instantiate inventory/targets and return Results. Specifically every task result should at least have the keys
status
andvalue
.