-
Notifications
You must be signed in to change notification settings - Fork 149
Apply tech author recommendations to repo docs #4384
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
|
Sorry, not sure why my commits aren't being signed. I've created an SSH key and added it within my GitHub settings. |
dave-gantenbein
left a comment
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.
Overall, a great improvement! I took a stab at answering your questions. Agreed that much more can be done here, but I'd consider taking this win and opening further PRs for new improvements.
docs/python_airflow_operator.md
Outdated
| title: Armada Airflow Operator | ||
| --- | ||
|
|
||
| # Armada Airflow Operator |
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.
Feel free to add that change here if you like.
| - [Armada Airflow Operator](#armada-airflow-operator) | ||
| - [armada.operators.armada module](#armadaoperatorsarmada-module) | ||
| - [_class_ armada.operators.armada.ArmadaOperator(name, channel\_args, armada\_queue, job\_request, job\_set\_prefix='', lookout\_url\_template=None, poll\_interval=30, container\_logs=None, k8s\_token\_retriever=None, deferrable=False, job\_acknowledgement\_timeout=300, dry\_run=False, reattach\_policy=None, extra\_links=None, \*\*kwargs)](#class-armadaoperatorsarmadaarmadaoperatorname-channel_args-armada_queue-job_request-job_set_prefix-lookout_url_templatenone-poll_interval30-container_logsnone-k8s_token_retrievernone-deferrablefalse-job_acknowledgement_timeout300-dry_runfalse-reattach_policynone-extra_linksnone-kwargs) | ||
| - [execute(context)](#executecontext) |
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 feels like a separate pull request to me.
|
Hey @dave-gantenbein, have just looked at your comments - they're great, thank you! Yeah, happy to get this merged and raise another PR to take care of any other bits. |
dave-gantenbein
left a comment
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.
Great improvements overall! I left a few comments addressing your final questions, which I think we can take up subsequently. Thanks for the contribution!
richscott
left a comment
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 had just one comment for you question about the missing launch.json.
|
@richscott Thanks for flagging that broken link! Have removed. If all looks ok, could I get an approval, please? |
Signed-off-by: EoinTrial <eoinsh@gmail.com>
Signed-off-by: EoinTrial <eoinsh@gmail.com>
Signed-off-by: EoinTrial <eoinsh@gmail.com>
Signed-off-by: EoinTrial <eoinsh@gmail.com>
richscott
left a comment
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.
LGTM.
This reverts commit 9830c4d. Signed-off-by: Geoff Wilson <geoff@gr-oss.io>
This reverts commit c6cf655. Signed-off-by: Geoff Wilson <geoff@gr-oss.io>
Updates contents of
docsfolder to: