skip_docker attribute added

ploomber / soopervisor

☁️ Export Ploomber pipelines to Kubernetes (Argo), Airflow, AWS Batch, SLURM, and Kubeflow.

https://soopervisor.readthedocs.io

Apache License 2.0

45 stars 18 forks source link

skip_docker attribute added #106

Closed yafimvo closed 2 years ago

yafimvo commented 2 years ago

Describe your changes

skip_docker attribute added to the export method in AbstractExporter and implemented in every exporter class (AWSBatchExporter, AirflowExporter, ArgoWorkflowsExporter, KubeflowExporter)

Issue ticket number and link

Closes #103

Checklist before requesting a review

[X] I have performed a self-review of my code
[X] I have added thorough tests (when necessary).
[X] I have added the right documentation (when needed). Product update? If yes, write one line about this update.

idomic commented 2 years ago

@yafimvo please fix failing tests

grnnja commented 2 years ago

I think short flags should be only one character so you can chain them together eg. tar -czvf for compressing. So maybe we use -S if -s is already used

idomic commented 2 years ago

good point, maybe -sd, -S might be a bit confusing.

edublancas commented 2 years ago

Agree with Prem that it should be one character. I think some of the short flags are two characters but that was a mistake on my end. If the -s is taken we can use -S

Sent from my iPad

On 31 Aug 2022, at 11:27, Prem Netsuwan @.***> wrote:

I think short flags should be only one character so you can chain them together eg. tar -czvf for compressing. So maybe we use -S if -s is already used

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you are subscribed to this thread.

idomic commented 2 years ago

Alright, let's go with -S then.

On Wed, Aug 31, 2022 at 1:57 PM Eduardo Blancas @.***> wrote:

Agree with Prem that it should be one character. I think some of the short flags are two characters but that was a mistake on my end. If the -s is taken we can use -S

Sent from my iPad

On 31 Aug 2022, at 11:27, Prem Netsuwan @.***> wrote:

I think short flags should be only one character so you can chain them together eg. tar -czvf for compressing. So maybe we use -S if -s is already used

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you are subscribed to this thread.

— Reply to this email directly, view it on GitHub https://github.com/ploomber/soopervisor/pull/106#issuecomment-1233246187, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACYPJONNLUGF2EQAC6V3QVLV36MJ5ANCNFSM57LA2M3Q . You are receiving this because you commented.Message ID: @.***>

edublancas commented 2 years ago

can we add a little note saying this was added in version 0.8.1?

also, the formatting for the link is broken: https://github.com/ploomber/soopervisor/blob/master/CHANGELOG.rst