-
Notifications
You must be signed in to change notification settings - Fork 110
Update workflows docs for changes in floe v0.17.0 and document builtin method parameters #1861
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
Conversation
e002279 to
08ef05e
Compare
46be44e to
7fe2243
Compare
2cb44f8 to
af078ac
Compare
|
Okay @Fryguy updated the parameters for http, email, embedded_ansbile, and provision_execute. |
| * `floe://http` - Execute any HTTP action | ||
|
|
||
| Parameters: | ||
| * `Method` (required) - HTTP method name. Permitted values: `GET`, `POST`, `PUT`, `DELETE`, `HEAD`, `PATCH`, `OPTIONS`, or `TRACE` |
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.
Not for now, but might be useful to default to GET
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.
ManageIQ/floe#315 but needs a floe release
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.
Depends on ManageIQ/manageiq-providers-workflows#145
| * `MinVersion` - Integer - Minimum SSL Version. | ||
| * `MaxVersion` - Integer - Maximum SSL Version. | ||
| * `Ciphers` - String - Ciphers supported. |
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.
We should default these to sensible defaults and doc the defaults (unless they are coming from elsewhere and that is too difficult)
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.
Farady doesn't list the default if unset, it just points to https://ruby-doc.org/stdlib-2.5.1/libdoc/openssl/rdoc/OpenSSL/SSL/SSLContext.html#method-i-ciphers-3D
| * `Encoding` - String | ||
| * `JSON` - JSON encodes the request and decodes the response |
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.
Is there a default value for when you don't pass it? Like, would it string String encoding? I know we don't literally set that under the covers, but I wonder if it should be a choice if the user wants to be more explicit.
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.
We only set json encoding if Encoding==JSON anything else passed has no effect.
| * `Encoding` - String | ||
| * `JSON` - JSON encodes the request and decodes the response | ||
| * `Proxy` | ||
| * `Uri` - String - URI of the proxy. |
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.
Should this be Url to mirror the other Url parameter?
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.
Faraday proxy options has uri specifically https://lostisland.github.io/faraday/#/customization/proxy-options and since that is technically more broad than a url I left it.
|
|
||
|  | ||
|
|
||
| ## Upgrading |
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.
My only thought is this should probably go into the blog post and not the docs. On downstream I could see it going into an Upgrade page. For now though I'm ok with merging this.
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.
Yeah we can drop it from here once we have release notes to add it to, didn't want to completely lose it until then.
a88e407 to
a272cb2
Compare
a272cb2 to
cf8a9e7
Compare
|
@jaychtw please also review. |
cf8a9e7 to
b70c9ac
Compare
b70c9ac to
72545ac
Compare
|
@jaychtw I'm going to merge for now to get this in. Any comments you have we can do in a follow-up PR. |
|
Backported to |
Update workflows docs for changes in floe v0.17.0 and document builtin method parameters (cherry picked from commit 258d815)
This updates the embedded_workflows docs to call out changes to Credentials handling and nested hash interpolation in floe 0.17.0
Also documents the builtin runner methods in floe and manageiq (floe://http was added in 0.17.0, we were missing any documentation on the manageiq:// ones so added those as well)