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

Added --overwrite-output (-O) option #716

Closed
wants to merge 3 commits into from
Closed

Conversation

yhager
Copy link
Contributor

@yhager yhager commented May 12, 2014

How about this for #547? This is my first time writing code for scrapy, so please scrutinize.

Fixes #547

@@ -67,13 +67,15 @@ class FileFeedStorage(object):
implements(IFeedStorage)

def __init__(self, uri):
from scrapy.conf import settings
Copy link
Member

@pablohoffman pablohoffman Jul 2, 2014

Choose a reason for hiding this comment

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

global import of settings is highly discouraged (and soon to be dropped), please switch to from_crawler class method (or similar)

yhager added 2 commits Jul 3, 2014
* used settings as passed by from crawler instead of input
* added unit tests for overwrite/append cases
@yhager
Copy link
Contributor Author

yhager commented Jul 3, 2014

from_crawler is not used for feed storage, but I managed to work around it. I hope it's not too intrusive.

self.path = file_uri_to_path(uri)
self.overwrite = settings['FEED_OVERWRITE']
Copy link
Contributor

@nramirezuy nramirezuy Jul 21, 2014

Choose a reason for hiding this comment

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

I think settings.getbool('FEED_OVERWRITE', default=False) fits better.

@kmike
Copy link
Member

kmike commented Mar 13, 2015

In the overview page (even after #1023) there is a gotcha: -o scraped_data.json will produce invaid JSON if called second time. We won't have to document this wart in the tutorial if this issue is fixed. So this feature is nice to have in Scrapy 1.0.

This PR looks like a good start. I think after addressing @nramirezuy's comments and rebasing on master it can be merged.

//cc @pablohoffman @curita @dangra

@abenassi
Copy link

abenassi commented Feb 3, 2016

Hi people, what's the status of this PR? I'm very interested in having this feature :-)

@kmike
Copy link
Member

kmike commented Feb 3, 2016

@abenassi yeah, that's a really nice feature. The PR needs a rebase. Another problem is that it changes IFeedStorage interface in a backwards-incompatible way; it'd be better to find a way to avoid doing that. There is also #1605, maybe it makes sense to finish it first. Help is appreciated - code review, suggestions, pull requests, rebasing existing pull requests :)

@abenassi
Copy link

abenassi commented Feb 3, 2016

sure @kmike I'd be interested in taking a look at Scrapy code to contribute! Although I'm new at it and I'll have to dive some time to understand how is the discussion going

@Gallaecio
Copy link
Member

Gallaecio commented Aug 5, 2019

It’s been a while, and rebasing may prove not that different from writing it from scratch once #3858 is merged. Shall we close?

@yhager
Copy link
Contributor Author

yhager commented Aug 6, 2019

I'm sorry but I am not using scrapy anymore, so don't have time/interest in rewriting this. Please someone else take over this, or you can close it as far as I'm concerned. Thanks!

@Gallaecio
Copy link
Member

Gallaecio commented Aug 8, 2019

I’ll continue the work myself once #3858 is merged.

@Gallaecio Gallaecio mentioned this pull request Apr 24, 2020
1 task
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.

Add a command-line option for overwriting exported file
6 participants