Skip to content

Comments

Add masterConfig parameter to MLEngineStartTrainingJobOperator#10578

Merged
mik-laj merged 7 commits intoapache:masterfrom
antoniocali:AAR-487-add-mlengine-master-config-parameter
Sep 4, 2020
Merged

Add masterConfig parameter to MLEngineStartTrainingJobOperator#10578
mik-laj merged 7 commits intoapache:masterfrom
antoniocali:AAR-487-add-mlengine-master-config-parameter

Conversation

@antoniocali
Copy link
Contributor

Jira
My PR addresses the following Airflow Jira issues and references them in the PR title.
https://issues.apache.org/jira/browse/AAR-487

Description
Why this is important?
At the moment when you spin up a training, you cannot specify an accelerator, so when you end up with a CUSTOM tier and a standard machine, you cannot add GPUs that would speed up the training process.

Documentation
It refers to the follow REST API Documentation.

Optional. The configuration for your master worker.

You should only set masterConfig.acceleratorConfig if masterType is set to a Compute Engine machine type. Learn about restrictions on accelerator configurations for training.


^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in UPDATING.md.

@boring-cyborg boring-cyborg bot added the provider:google Google (including GCP) related issues label Aug 26, 2020
@boring-cyborg
Copy link

boring-cyborg bot commented Aug 26, 2020

Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contribution Guide (https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst)
Here are some useful points:

  • Pay attention to the quality of your code (flake8, pylint and type annotations). Our pre-commits will help you with that.
  • In case of a new feature add useful documentation (in docstrings or in docs/ directory). Adding a new operator? Check this short guide Consider adding an example DAG that shows how users should use it.
  • Consider using Breeze environment for testing locally, it’s a heavy docker but it ships with a working Airflow and a lot of integrations.
  • Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
  • Please follow ASF Code of Conduct for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
  • Be sure to read the Airflow Coding style.
    Apache Airflow is a community-driven project and together we are making it better 🚀.
    In case of doubts contact the developers at:
    Mailing List: dev@airflow.apache.org
    Slack: https://apache-airflow-slack.herokuapp.com/

@antoniocali
Copy link
Contributor Author

It seems spell checks failed but I don't recognize where is the error. If someone would help me with this

@mik-laj
Copy link
Member

mik-laj commented Aug 28, 2020

Can you add tests to avoid regression?

@antoniocali antoniocali force-pushed the AAR-487-add-mlengine-master-config-parameter branch from e3fa9b5 to 64981bc Compare September 2, 2020 11:21
@antoniocali antoniocali force-pushed the AAR-487-add-mlengine-master-config-parameter branch from 1634a3f to dda2335 Compare September 3, 2020 07:17
@mik-laj mik-laj merged commit 6e3d7b6 into apache:master Sep 4, 2020
@boring-cyborg
Copy link

boring-cyborg bot commented Sep 4, 2020

Awesome work, congrats on your first merged pull request!

@antoniocali antoniocali deleted the AAR-487-add-mlengine-master-config-parameter branch September 6, 2020 10:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

provider:google Google (including GCP) related issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants