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

Implement Azure ML Service tasks #1590

Merged
merged 4 commits into from Oct 3, 2019
Merged

Conversation

frsann
Copy link

@frsann frsann commented Oct 3, 2019

Thanks for contributing to Prefect!

Please describe your work and make sure your PR:

  • adds new tests (if appropriate)
  • updates CHANGELOG.md (if appropriate)
  • updates docstrings for any new functions or function arguments, including docs/outline.toml for API reference docs (if appropriate)

Note that your PR will not be reviewed unless all three boxes are checked.

What does this PR change?

It adds tasks to work with Azure Machine Learning Service, and more specifically Datasets and Datastores.

Why is this PR important?

Helps Azure ML service users to register and update datastore and datasets from Prefect.

@marvin-robot
Copy link
Member

Here I am, brain the size of a planet and they ask me to welcome you to Prefect.

So, welcome to the community @frsann! 🎉 🎉

@frsann frsann force-pushed the azureml-tasks branch 3 times, most recently from e59ed6b to 606d69c Compare October 3, 2019 10:51
@codecov
Copy link

codecov bot commented Oct 3, 2019

Codecov Report

Merging #1590 into master will increase coverage by 0.01%.
The diff coverage is 94.93%.

Copy link

@joshmeek joshmeek left a comment

Choose a reason for hiding this comment

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

This looks really good! Quick question before I go in for a deeper review

- datastore_name (str, optional): The name of the datastore. If not defined, the container name will be used.
- create_container_if_not_exists (bool, optional): Create a container, if one does not exist with the given name.
- overwrite_existing_datastore (bool, optional): Overwrite an existing datastore. If the datastore does not exist, it will be created.
- azure_credentials_secret (str, optinonal): The name of the Prefect Secret that stores your Azure credentials; this Secret must be a JSON string with two keys: `ACCOUNT_NAME` and `ACCOUNT_KEY`
Copy link

@joshmeek joshmeek Oct 3, 2019

Choose a reason for hiding this comment

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

What's the background behind this being the only task that requires azure credentials?

Copy link
Author

Choose a reason for hiding this comment

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

The authentication to the Azure ML service is done through the Workspace-object.

In the case you point out, we are registering/linking a Blob Container to the Workspace, and the we're authenticating to the Blob storage, not the Workspace.

The authentication in Blob storage is different from Azure ML service (and managing resources in Azure more generally). See this Jupypter notebook for an example.

Copy link

Choose a reason for hiding this comment

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

Okay that makes a lot more sense, thanks for explaining and linking the notebook!

@joshmeek
Copy link

joshmeek commented Oct 3, 2019

@frsann It seems like your commits aren't linked to you as a user (https://help.github.com/en/articles/why-are-my-commits-linked-to-the-wrong-user#commits-are-not-linked-to-any-user) and show up as the grey Octocat. Would you like to adjust that and reopen this PR?

@cicdw
Copy link
Member

cicdw commented Oct 3, 2019

I'm going to merge this now so that it goes out in the next release!

@cicdw cicdw merged commit 25a52c5 into PrefectHQ:master Oct 3, 2019
@frsann
Copy link
Author

frsann commented Oct 4, 2019

@joshmeek Thanks for the heads up! I definitely want my GitHub points! 😄 Fixed the problem by linking my other email to my Github account.

zanieb pushed a commit that referenced this pull request Apr 13, 2022
UI: Use a button to add filters rather than automatically adding new rows
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants