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

generate a _deleted.txt file in CLI #2589

Closed
wants to merge 1 commit into from

Conversation

peterbe
Copy link
Contributor

@peterbe peterbe commented Jan 19, 2021

Part of #2224

When you use:

yarn tool delete Glossary/PNG

Generates (in files/en-us/_deleted.txt):

# This file is generated when you use the CLI tool to delete slugs.

/en-US/docs/Glossary/PNG                                     # Tue Jan 19 2021

And it works recursively and outside CONTENT_ROOT too:

yarn tool delete Glossary sv-SE -y -r

Generates (in files/sv-se/_deleted.txt):

# This file is generated when you use the CLI tool to delete slugs.

/sv-SE/docs/Glossary/404                                     # Tue Jan 19 2021
/sv-SE/docs/Glossary/Bandwidth                               # Tue Jan 19 2021
/sv-SE/docs/Glossary/IP_Adress                               # Tue Jan 19 2021
/sv-SE/docs/Glossary                                         # Tue Jan 19 2021

Now, this means we can start reading from this file, in the Deployer, before we proceed to upload stuff. This way, stuff will disappear much sooner from S3 and Elasticsearch.

@peterbe peterbe requested a review from fiji-flo January 19, 2021 14:49
@peterbe
Copy link
Contributor Author

peterbe commented Jan 19, 2021

CC @escattone

Copy link
Contributor

@fiji-flo fiji-flo left a comment

Choose a reason for hiding this comment

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

One nit. Bigger question is if we want to use a delete file approach or a "we gather everything and calculate the difference" approach.

"to delete slugs.\n\n";
}
const url = buildURL(locale, slug);
const newLine = `${url.padEnd(60, " ")} # ${new Date().toDateString()}\n`;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const newLine = `${url.padEnd(60, " ")} # ${new Date().toDateString()}\n`;
const newLine = `${url.padEnd(60, " ")}\t# ${new Date().toDateString()}\n`;

can we use a tab here? Easier to split and process.

@peterbe
Copy link
Contributor Author

peterbe commented Jan 21, 2021

One nit. Bigger question is if we want to use a delete file approach or a "we gather everything and calculate the difference" approach.

Yeah, let's hold off on this PR.
The list of archived.txt URLs plus other strategies for the Elasticsearch publishing and stuff might just be enough. And if so, no need for this.

@peterbe peterbe marked this pull request as draft January 21, 2021 17:50
@escattone
Copy link
Contributor

One nit. Bigger question is if we want to use a delete file approach or a "we gather everything and calculate the difference" approach.

Yeah, let's hold off on this PR.
The list of archived.txt URLs plus other strategies for the Elasticsearch publishing and stuff might just be enough. And if so, no need for this.

I had the same question as @fiji-flo, so I like that we're holding off for now and sleep on this some more.

Base automatically changed from master to main February 16, 2021 16:38
@peterbe
Copy link
Contributor Author

peterbe commented Feb 23, 2021

We decided that the Deployer can figure out what should be deleted based on the archived.txt instead.

@peterbe peterbe closed this Feb 23, 2021
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