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

.amlignore example of usage #155

Merged
merged 8 commits into from
Jan 30, 2020
Merged

.amlignore example of usage #155

merged 8 commits into from
Jan 30, 2020

Conversation

sbaidachni
Copy link
Contributor

No description provided.

@dtzar
Copy link
Member

dtzar commented Jan 29, 2020

This makes sense, however at the directory you put the file there are no .yml files underneath it. If you put it at the root it would make sense if someone specified the root source directory to be transferred to remote compute. How about you put it at the root dir and ignore all the directories and file extensions not relevant?

I'd also link to a doc/site where it talks about .amlignore.

@dtzar dtzar changed the title .amlignore has been added as an example of usage .amlignore example of usage Jan 29, 2020
@sbaidachni
Copy link
Contributor Author

@dtzar There are at least 4 yml files in scoring subfolder:) The root folder doesn't make any sense, because you submit diabetes folder to aml compute. So, everything that is outside the diabetes folder should be ignored by the PythonScriptStep including amlignore:)

@@ -0,0 +1,3 @@
# We use yml files to configure deployment,
# but we are not deploying them to compute
Copy link
Member

Choose a reason for hiding this comment

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

Please put a better description of what this does here IF there is no link to a doc with what this does.

@dtzar
Copy link
Member

dtzar commented Jan 30, 2020

Ok, no problem on the location & .yml file specification. Would be nice to know how people can ignore a directory (even if it isn't present under there) IF there is no public document explaining it. So... with a public link you should be able to just comment with the link for more info. No link, then you need better description + folder exclusion example.

@sbaidachni
Copy link
Contributor Author

Will it be OK if I just add a note that the format of the file is identical to .gitignore?

@dtzar
Copy link
Member

dtzar commented Jan 30, 2020

Yes but I remember @xinyi-joffre stating something like there are some quirks to .amlignore where it doesn't behave exactly like this? Is there ANY documentation referencing .amlignore or you just need tribal knowledge?

@xinyi-joffre
Copy link

The documentation about .amlignore is usually in notes in the docs. For example, here. It may also be worthwhile to leave a comment about how this file can be used to reduce snapshot size if you get errors about too many files in snapshot or if it surpasses the max snapshot size.

The .amlignore should theoretically behave the same way as .gitignore. When we initially used it, the underlying implementation was broken for the exclude all pattern where you start with * and then add files back into snapshot. Also, we had to make sure that we included the .amlignore itself in the snapshot. We reported it to product team, and according to this post, the exclude all bug was fixed in 1.0.74 of azureml-sdk, but I haven't tested to make sure it works.

Azure ML has their own logic for building a tree of the hash paths so there is still a possibility of it unintentionally behaving slightly differently than .gitignore.

If you are only ignoring files directly (such as *.yml), it should work well.

@dtzar
Copy link
Member

dtzar commented Jan 30, 2020

Thanks @xinyi-joffre! I think the link you provided is sufficient for the comment :)

@sbaidachni sbaidachni merged commit a24870a into master Jan 30, 2020
@sbaidachni sbaidachni deleted the sbaydach)_aml_ignore branch January 30, 2020 23:02
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.

3 participants