Skip to content
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

update azure installation guide #2196

Merged
merged 9 commits into from
Oct 20, 2020

Conversation

sudivate
Copy link
Contributor

CC- @aronchick

@google-cla
Copy link

google-cla bot commented Sep 17, 2020

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@kubeflow-bot
Copy link

This change is Reviewable

@sudivate
Copy link
Contributor Author

/assign @aronchick

Copy link
Contributor

@8bitmp3 8bitmp3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @sudivate , thanks for the update to Kubeflow deployment on Azure.

I've made a few suggestions.

  • Line 16: Would you consider adding the official Docker docs too, such as https://docs.docker.com/docker-for-windows/wsl/? A blog post is OK but less official IMHO.

  • Line 17:

      - For other OS: [Docker Desktop](https://hub.docker.com/?overlay=onboarding)
    

    I think this Docker Hub link would take the user straight to a login page, which is not awesome in terms of UX, especially if they don't have an account yet. Maybe, a page from https://docs.docker.com/docker-hub/ would suffice?

  • Lines 55-56: Add a quick sentence and back ticks to the code block with command lines:

+ To log into Azure from the command line interface, run the following commands:

+     ```
    az login
    az account set --subscription <NAME OR ID OF SUBSCRIPTION>
+     ```
  • Line 62: You need this code block to be surrounded by back ticks, for instance:
+     ```
    az group create -n <RESOURCE_GROUP_NAME> -l <LOCATION>
+     ```
  • Line 71: You need this code block to be surrounded by back ticks, for instance:
+     ```
    az aks create -g <RESOURCE_GROUP_NAME> -n <NAME> -s <AGENT_SIZE> -c <AGENT_COUNT> -l <LOCATION> --generate-ssh-keys
+     ```
  • Lines 75-76: Formatting - back ticks:
- `NAME=KubeTestCluster`
- `AGENT_SIZE=Standard_D4s_v3`
  • Line 80: Usually, on the Kubeflow site, the Note: paragraph doesn't have to be all bold. Also, "GPU-based".
**Note**:  If you are using...
  • Line 146: add a definite article "the":
1. Open the Kubeflow Dashboard
  • Line 148: surround the site name with back ticks instead of <>:
The default installation does not create an external endpoint but you can use port-forwarding to visit your cluster. Run the following command and visit `http://localhost:8080`.

LMKWYT.

Thanks!

content/en/docs/azure/deploy/install-kubeflow.md Outdated Show resolved Hide resolved
content/en/docs/azure/deploy/install-kubeflow.md Outdated Show resolved Hide resolved
content/en/docs/azure/deploy/install-kubeflow.md Outdated Show resolved Hide resolved
content/en/docs/azure/deploy/install-kubeflow.md Outdated Show resolved Hide resolved
content/en/docs/azure/deploy/install-kubeflow.md Outdated Show resolved Hide resolved
content/en/docs/azure/deploy/install-kubeflow.md Outdated Show resolved Hide resolved
content/en/docs/azure/deploy/install-kubeflow.md Outdated Show resolved Hide resolved
content/en/docs/azure/deploy/install-kubeflow.md Outdated Show resolved Hide resolved
@sudivate sudivate requested a review from 8bitmp3 September 17, 2020 21:14
@sudivate sudivate requested a review from 8bitmp3 September 17, 2020 21:45
@sudivate
Copy link
Contributor Author

sudivate commented Sep 17, 2020

@8bitmp3 if you see the netlify preview formatting under "Unpack the tarball:" is broken post-deployment , any pointers what's missing here? However local rendering is perfect

@8bitmp3
Copy link
Contributor

8bitmp3 commented Sep 17, 2020

Thank you for your efforts @sudivate

I've just submitted a PR that creates a new OWNERS file to help with Azure docs review process #2199 - it turns out KF Azure hasn't had official OWNERS assigned yet.

@aronchick Let us know what you think about the changes here and in #2199

Cheers

@sudivate
Copy link
Contributor Author

@8bitmp3 while we get the approvers added for Azure who can be the provisional approver for this PR?

@sudivate
Copy link
Contributor Author

/assign @eedorenko

@eedorenko
Copy link

/lgtm
/approve

@eedorenko
Copy link

/assign @animeshsingh

@sudivate
Copy link
Contributor Author

@animeshsingh can I please get your approval?

@Bobgy
Copy link
Contributor

Bobgy commented Oct 20, 2020

/lgtm
/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Bobgy, eedorenko, sudivate

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot merged commit 582466c into kubeflow:master Oct 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants