Repository: scrapy/scrapy
Pull request: Check if file is already present on running `scrapy genspider` and terminate if so (#4561)
Continuing from PR #4616 to fix #4561, I've added a direct file check to see if a spider with the name provided in `scrapy genspider` is already present in the directory and not overwrite the existing spider file if present.

Problem statement:
`scrapy genspider` should not overwrite existing file

## Summary

If the file mentioned in `scrapy genspider` already exists, then genspider should refuse to generate the the file.

## Motivation

As it stands, existing code can be blown away if this command runs twice.

## Describe alternatives you've considered

Prompting the user for overwriting existing spider.

Hints from issue discussion:
Hey, is this Issue up for grabs??
As far as I am concerned, yes.

On Mon, May 11, 2020, 4:02 AM Jay Rajput <notifications@github.com> wrote:

> Hey, is this Issue up for grabs??
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> <https://github.com/scrapy/scrapy/issues/4561#issuecomment-626540486>, or
> unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAAFGLK7MXQJNHDX4I4AB2DRQ6WIHANCNFSM4M5FRRDQ>
> .
>

@Gallaecio @elacuesta thoughts??
I would not have thought this was an issue, as I'd expect spider code to be covered by source control. On the other hand, it should be only a matter of doing a file system check, so I think a clean implementation could be approved.
Alright! I will work on this one
Hi @metaperl @elacuesta,

I would like to understand this properly. I've tried to create a spider with the same name twice but it does not allow me to do so. 

```
(p3env) bash-3.2$ scrapy genspider toi 'https://timesofindia.indiatimes.com/'
Created spider 'toi' using template 'basic' in module:
  testscrapy.spiders.toi
(p3env) bash-3.2$
(p3env) bash-3.2$
(p3env) bash-3.2$ scrapy genspider toi 'https://timesofindia.indiatimes.com/'
Spider 'toi' already exists in module:
  testscrapy.spiders.toi
```

Could you please let me know what exactly is the issue so that I can try attempting to resolve or enhance?

Thanks & Regards.

Commit message: Add test to ensure spider is not overwritten if present

Patch to review:
diff --git a/scrapy/commands/genspider.py b/scrapy/commands/genspider.py
index abf3b7a5c68..638c8a65b3d 100644
--- a/scrapy/commands/genspider.py
+++ b/scrapy/commands/genspider.py
@@ -66,6 +66,10 @@ def run(self, args, opts):
             print("Cannot create a spider with the same name as your project")
             return
 
+        if exists(name + ".py"):
+            print("Spider %r already exists in module" % name)
+            return
+
         try:
             spidercls = self.crawler_process.spider_loader.load(name)
         except KeyError:
diff --git a/tests/test_commands.py b/tests/test_commands.py
index 24a341759b7..2b2bdcd0f71 100644
--- a/tests/test_commands.py
+++ b/tests/test_commands.py
@@ -188,6 +188,10 @@ def test_same_name_as_project(self):
         self.assertEqual(2, self.call('genspider', self.project_name))
         assert not exists(join(self.proj_mod_path, 'spiders', '%s.py' % self.project_name))
 
+    def test_same_name_as_existing_spider(self):
+        self.call('genspider', 'example', 'example.com')
+        self.assertEqual(0, self.call('genspider', 'example', 'example.com'))
+
 
 class GenspiderStandaloneCommandTest(ProjectTest):
 


Code review comments:
diff_hunk: @@ -66,6 +66,10 @@ def run(self, args, opts):
             print("Cannot create a spider with the same name as your project")
             return
 
+        if exists(name + ".py"):
+            print("Spider %r already exists in module" % name)
path: scrapy/commands/genspider.py
text: @user1:
This message is not entirely accurate, as any file with the same name would trigger it (not necessarily a spider).

@author:
Do you think a message like "File %r.py already exists in the current directory" would be more accurate? and I think allowing it to overwrite the existing file if the --force option is used would be suitable as described in the original issue, what do you think?

@user1:
I think talking about the file is indeed better, thanks for making the change. I don't think adding a `--force` option is necessary, in my opinion it'd just make the code more complex for something that could be solved easily outside of Scrapy (renaming the existing file or choosing a different name for the new file).

@author:
I agree, so I guess no more changes here then?