feat: Initialize a project from non-cookiecutter github repo or zip file #1595
Conversation
| @@ -2,25 +2,4 @@ | |||
| Helper functions for temporary files | |||
| """ | |||
| import os | |||
There was a problem hiding this comment.
I moved these to osutils.py to consolidate all OS related helpers. This is not required for this PR, but I thought it might be a good cleanup to do while I am here
| @@ -33,13 +37,13 @@ def test_init_command_passes_and_dir_created(self): | |||
| raise | |||
|
|
|||
| self.assertEqual(process.returncode, 0) | |||
| self.assertTrue(os.path.isdir(temp + "/sam-app")) | |||
| self.assertTrue(Path(temp, "sam-app").is_dir()) | |||
There was a problem hiding this comment.
Refactored to remove hardcoded / while I am here
| try: | ||
| # Let's try to copy the directory metadata from source to destination | ||
| shutil.copystat(source, destination) | ||
| except OSError as ex: |
There was a problem hiding this comment.
this seems broad to be just windows errors, can we get more granular?
There was a problem hiding this comment.
this is not even my code. it's copy pasted ;)
| if name in ignored_names: | ||
| continue | ||
|
|
||
| new_source = os.path.join(source, name) |
There was a problem hiding this comment.
Nit: I see some parts of the code changing to use pathlib, do we need to do the same here?
There was a problem hiding this comment.
eventually. I don't want to change much now
|
|
||
| @parameterized.expand([(None,), ("project_name",)]) | ||
| def test_arbitrary_project(self, project_name): | ||
| with tempfile.TemporaryDirectory() as temp: |
There was a problem hiding this comment.
are there any gotchas with temporary directories on windows?
There was a problem hiding this comment.
it didn't break before ;) so no
| try: | ||
| os.remove(path) | ||
| except OSError: | ||
| pass |
There was a problem hiding this comment.
This will fail if it throws IsADirectoryError.
There was a problem hiding this comment.
it's only used in a test. I think we should clean this up later.
|
|
||
| # NOTE: Py3.8 or higher has a ``dir_exist_ok=True`` parameter to provide this functionality. | ||
| # This method can be removed if we stop supporting Py37 | ||
| def copytree(source, destination, ignore=None): |
There was a problem hiding this comment.
This method is well documented 😉 others are not.
There was a problem hiding this comment.
because this was copied from lambda-builders :)
| if repository.is_zip_file(location): | ||
| LOG.debug("%s location is a zip file", location) | ||
| download_fn = functools.partial( | ||
| repository.unzip, zip_uri=location, is_url=repository.is_repo_url(location), no_input=no_input | ||
| ) | ||
|
|
||
| # Else, treat it as a git/hg/ssh URL and try to clone | ||
| elif repository.is_repo_url(location): | ||
| LOG.debug("%s location is a source control repository", location) | ||
| download_fn = functools.partial(repository.clone, repo_url=location, no_input=no_input) |
There was a problem hiding this comment.
Just curious, if cookiecutter supports zip and git both, then why do we need two if conditions?
There was a problem hiding this comment.
because cookiecutter's internal code verifies the presence of cookiecutter.json. we don't want that
Issue #, if available:
Description of changes:
Checklist:
make prpassesBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.