15 ms·
> but at the time the code seemed completely correct to me It always does. > Well, it teaches me to do more diverse tests when doing destructive operations.
by dsego 5y ago
> but at the time the code seemed completely correct to me
It always does.
> Well, it teaches me to do more diverse tests when doing destructive operations.
Or add some logging and do a dry run and check the results, literally simple prints statements:
print("-----")
print("Downloading videos ids from url: {url}")
print(list of ids)
...
...
...
# delete() dangerous action commented out until I'm sure it's right
print("I'm about to delete video {id}")
print("Deleted {count} videos") # maybe even assert
...
Then dump out to a file and spot check it five times before running for real.
- V__ 5y agoThis was my first thought too. Another think I like to do, is to limit the loop to say one page or 10 entries and check after each run that it was correctly executed. It makes it a half-automated task, but saves time in the long run.
- mipmap04 5y agoI do this, too, but I also take a count of the expected number of items to be deleted as well. If my collection I'm iterating over doesn't have exactly that number of objects I expect, I don't proceed.
- gilleain 5y agoYes, I find command line tools that have a "--dry-run" flag to be very helpful. If the tool (or script or whatever) is performing some destructive or expensive change, then having the ability to ask "what do you think I want to do?" is great. It's like the difference between "do what I say" and "do what I mean"...
- mmcclimon 5y agoThe rule we have is that anything that is not idempotent and not run as a matter of daily routine must dry-run by default, and not take action unless you pass --really. This has saved my bacon many times!
- maweki 5y agoDeleting actually is idempotent. Doing it twice wont be different from doing it once.
- maccard 5y agoDeleting * may not be though. Your selection needs to be idempotent.
- maweki 5y agoidempotency means that f(X) = f(f(X)). Modifying the X inbetween is not allowed. Is there really an initial environment where rm * ; rm * ; does something different than rm * once?
- einsty 5y agoIn the case of any live system, i would say yes. Additional, and different, files could have appeared on the file system in between the times of each rm *.
- mikeryan 5y ago* is just short hand for a list of files. Calling rm with the same list of files will have the same results if you call it multiple times. That’s idempotent. Your example is changing the list of files, or arguments to rm between runs. Same as pc85’s example where the timestamp argument changes.
- pc86 5y agoIn addition to what einsty said (which is 100% accurate), if you're deleting aged records, on any system of sufficient size objects will become aged beyond your threshold between executions.
- jameshart 5y ago
- rjh29 5y agoGoing further, make it dry run by default and have an --execute flag to actually run the commands: this encourages the user to check the dryrun output first.
- bzxcvbn 5y agoThat's what I like about powershell. Every script can include a "SupportsShouldProcess" [1] attribute. What this means is that you can pass two new arguments to you script, which have standardized names across the whole platform: - -WhatIf to see what would happen if you run the script; - -Confirm, which asks for confirmation before any potentially destructive action. Moreover these arguments get passed down to any command you write in your script that support them. So you can write something like: [CmdletBinding(SupportsShouldProcess)] param ([Parameter()] [string] $FolderToBeDeleted) # I'm using bash-like aliases but these are really powershell cmdlets! echo "Deleting files in $FolderToBeDeleted" $files = @(ls $FolderToBeDeleted -rec -file) echo "Found $($files.Length) files" rm $files If I call this script with -WhatIf, it will only display the list of files to be deleted without doing anything. If I call it with -Confirm, it will ask for confirmation before each file, with an option to abort, debug the script, or process the rest without confirming again. I can also declare that my script is "High" impact with the "ConfirmImpact = High" switch. This will make it so that the user gets asked for confirmation without explicitly passing -Confirm. A user can set their $ConfirmPreference to High, Medium, Low, or None, to make sure they get asked for confirmation for any script that declare an impact at least as high as their preference. [1]: https://docs.microsoft.com/en-us/powershell/scripting/learn/deep-dives/everything-about-shouldprocess?view=powershell-7.2 https://docs.microsoft.com/en-us/powershell/scripting/learn/...
- spookthesunset 5y agoI’m a bit confused (because I didnt read the docs)… does calling it with “—whatif” exercise the same code path as calling without, only the “do destructive stuff” automagically doesn’t do anything? Or is it a separate routine that you have to write? Cause if it is an entirely separate code path, doesn’t that introduce a case where what you say you’ll isn’t exactly what actually happens?
- bzxcvbn 5y agoIt's the first option. And yes, sometimes you have to be careful if you want to implement SupportsShouldProcess correctly, it's not something you can add willy-nilly. For example, if you create a folder, you can't `cd` there in -WhatIf mode.
- FriedrichN 5y agoAll my tools that have a possible destructive outcome use either a interactive stdin prompt or a --live option. I like the idea of dry running by default.
- inglor_cz 5y agoYeah, that is what I recommend too. Instead of performing the dangerous action outright, just log a message to screen (or elsewhere) and watch what is happening. Alternatively, or subsequently, chroot and try that stuff on some dummy data to see if it actually works.
- dkersten 5y agoI was involved with archiving of data that was legally required to be retained for PSD2 compliance. So it was pretty important that the data was correctly archived, but it was just as important that it was properly removed from other places due to data protection. This is basically the approach that was taken: log before and after every action exactly what data or files is being acted on and how. Don't actually do it. Then have multiple people inspect the logs. Once ok'd, run again, with manual prompts after each log item asking to continue, for the first few files/bits of data. Only after that was ok'd too did it run the remainder. In other things I've worked on, I've taken the terraform-style plan first, then apply the plan approach, with manual inspection of the plan in between.
- zeristor 5y agoYes, I love the idea of the Plan Apply.
- tauwauwau 5y agoOnce we get used to doing same thing multiple times a day, it doesn't matter if the log shows that we're about to take a destructive action, we'll still do it. Only thing that is foolproof is to not take the destructive action because people make mistake, it's human nature. I don't know how this can be implemented, may be encrypt the files, take a backup in some other location (which may not be allowed). Multiple reviewers here didn't catch the mistake https://www.bloombergquint.com/markets/citi-s-900-million-misfire-happened-in-midst-of-software-switch https://www.bloombergquint.com/markets/citi-s-900-million-mi...
- dkersten 5y ago> Multiple reviewers here didn't catch the mistake Sure, but we can only do so much. I find its good bang for buck and alternatives that might prevent that are not always available, so we do the best we can. You gotta make a call on whether its enough or not.
- Danieru 5y agoThat form had a couple weird checkboxes with odd wording. It is a famous mistake, but also rather understandable just because the form was cryptic.
- veltas 5y agoYep, even writing a simple wildcard at command-line I will 'echo' before I 'rm'.
- pjerem 5y agoOn computers I own, I always install "trash-cli" and i even created an alias for rm to trash. It's like rm, but it goes to the good old trash. It will not save your prod but it's pretty useful on your own computer at least.
- cryptoboid 5y agoThat's a good tip, thanks!
- jagged-chisel 5y agoThis is how I do it in compiled code. In shell, I print the destructive command for dry runs - no conditions around whether to print or not, I go back to remove echo and printf to actually run the commands.
- aqme28 5y agoRather than commenting it out, I suggest adding a --live-run flag to scripts and checking the output of --live-run=false (or omitted) before you run it "live."
- sdevonoes 5y agoBut then you have double the chances of introducing a bug for the specific scenario we are talking about: Before: there is chance there is a bug in my "delete" use case Now: what we have before plus the change that there is a bug in my "--live-run" flag
- aqme28 5y agoYou can make automated tests for your flag. You can’t make automated tests for your code comments.
- lifthrasiir 5y agoHuman-in-the-loop is so important concept in ops and yet everyone (that's including me) seems to learn it the hard way.
- password4321 5y agoSELECT COUNT(1) FROM table -- UPDATE table SET col='val' WHERE 1=1
- worble 5y agoBEGIN TRANSACTION UPDATE table SET col='val' WHERE 1=1 ROLLBACK
- tomrod 5y agoExactly!
- password4321 5y agoDefinitely better, when you can afford the overhead!
- OrwellianTimes 5y agoExperience is the best teacher™
- bambax 5y agoYes. Also, maybe not have a delete action in the middle of a script. It's usually better to build a list of items to be deleted. In that case, two lists: items to be deleted, items to be kept. Then compare the lists: - make sure the sum of their lengths == number of total current items - make sure items_to_be_kept.length != 0 - make sure no two items appear in both lists - check some items chosen at random to see if they were sorted in the correct list At this point the only possible mistake left is to confuse the lists and send the "to_be_kept" one to the delete script; a dry run of the delete list can be in order.
- pc86 5y agoI've had good success with this approach, have two distinct scripts generate the two lists, then in addition to your items here also checking that every item appears in one of the lists.
- ufo 5y agoWhat do you recommend, to not get intro trouble if there are spaces or newlines in the file names?
- kevinmgranger 5y agoDon't use a shell script.
- ufo 5y agoDo you mean, always pass the list directly to the next script via function calls, without writing it to an intermediate file / pipeline?
- plonk 5y agoYes, use the list argument to Python’s subprocess.run for example. It’s much easier to not mess up if your arguments don’t get parsed by a shell before getting passed.
- 4y ago
- mkr-hn 5y agoThis sounds like a "do nothing script." https://news.ycombinator.com/item?id=29083367 https://news.ycombinator.com/item?id=29083367 It defaults to not doing anything so you can gradually and selectively have it do something. Learned about when I posted my command line checklist tool on HN: https://github.com/givemefoxes/sneklist https://github.com/givemefoxes/sneklist (https://news.ycombinator.com/item?id=25811276 https://news.ycombinator.com/item?id=25811276) You could use it to summon up a checklist of to-dos like "make sure the collection in the dictionary has the expected number of values" before a "do you want to proceed? Y/n"
- zrail 5y agoAnother technique that I've used with good success is to write a script that dumps out bash commands to delete files individually. I can visually inspect the file, analyze it with other tools, etc and then when I'm happy it's correct just "bash file_full_of_rms.sh" and be confident that it did the right thing.
- cruano 5y agoThat was our SOP for running DELETE SQL commands on production too, a script that generates a .sql that's run manually. It saved out asses a fair amount of times
- ineedasername 5y agoYeah, wish I'd learned that the easy way. Fresh into one of my first jobs I was working with a vendor's custom interface to merge/purge duplicate records. It didn't have a good method of record matching on inserts from the customer web interface so a large % of records had duplicates. Anyway, I selected what I though was a "merge all duplicates" option without previewing results. What I had actually done was "merge all selected". So, the system proceeded to merge a very large % of the database... Into One. Single. Record. Luckily the vendor kept very good backups, and so I kept my job. Because I also luckily had a very good boss and I had already demonstrated my value in other ways, he just asked me "Well, are you going to make that mistake again?". I wisely said no, and he just smiled and said "Then I think we're done here." I have been particularly fortunate throughout my career to have very good managers. As much as managers get a lot of flack here on HN, done well they are empowering, not a hindrance, and I attribute a lot of success in my career to them.
- JadeNB 5y ago> Yeah, wish I'd learned that the easy way. I think that, if you've only learned something like that the easy way, then you haven't learned it yet. As long as everything's only ever gone right, it's easy to think, I'm in a rush this one time, and I've never really needed those safety procedures before, ….
- 5y ago
- pc86 5y agoI just want to say as someone currently working on a script to delete approximately 3.2TB of a ~4TB production database, this subthread is pure gold.
- ineedasername 5y ago>literally simple prints statements Yes, that can be a simple but powerful live on screen log. I developed a library to use an API from a SaaS vendor, in much the same way as the author. It was my first such project & I learned the hard way (wasted time, luckily no data loss or corruption) that print() was an excellent way to keep tabs on progress. On more than one occasion it saved me when the results started scrolling by and I did an oh sh*t! as I rushed to kill the job.
- krono 5y agoThe No. 2 philosophy! Make sure you got everything out and off before you pull up your pants, or else you better be prepared to deal with all the shit that might follow!
- GordonS 5y agoIt's amazing the number of times I look at some simple code and think "nah, this is so simple it doesn't need a test!", add tests anyway (because I know I should)... and immediately find the test fails because of an issue that would have been difficult to diagnose in production. Automated tests are awesome :)
- dncornholio 5y agoDry run really is key here. Most automated tests wouldn't find this bug.
- hayd 5y agoI'd make sure those include WARN or ERROR (I'd use logging to do that), that way you can grep for those. Spot checking might be difficult if the logs get long.
- thunderbong 5y agoThat is called experience. Good decisions come from experience. Experience comes from making bad decisions.
- mbiondi 5y agoAgreed, I've also been burned doing stupid things like this and always print out the commands and check them before actually doing the commit. As they say, measure twice, cut once. Don't feel bad, I think every professional in IT goes through something similar at one time or another.
- rawgabbit 5y agoTo ensure that the files are actually are downloaded (step1), before deleting the original (step2). I would make make step1 an input to step2. That is step2 cannot work without step1. Something like: (step1) Download video from URL. Include the Id in the filename. (step2) Grab the list of files that have been downloaded and parse to get the Id. Using the Id, delete the original file.
- kortex 5y agoThis is why I like to always write any sort of user-script batch-job tools (backfills, purges, scrapers) with a "porcelain and plumbing" approach: The first step generates a fully declarative manifest of files/uris/commands (usually just json) and the second step actually executes them. I've used a --dry-run flag to just output the manifest, but I just read some folks use a --live-run flag to enable, with dry-run being the default, and I like that much better so I'll be using that going forward. This pattern has the added benefit that it makes it really easy to write unit tests, which is something often sorely lacking in these sorts of batch scripts. It also makes full automation down the line a breeze, since you have nice shearing layers between your components. http://www.laputan.org/mud/mud.html#ShearingLayers http://www.laputan.org/mud/mud.html#ShearingLayers
- InfoSecErik 5y agoI tend towards a --dry-run flag for creative actions and --confirm for destructive actions. Probably sightly annoying that the commands end up seemingly different, but it sure beats accidentally nuking something important.
- hinkley 5y agoCondensed to aphorism form: Decide, then act. There's a whole menagerie of failure modes that come from trying to make decisions and actions at the same time. This is but one of them. Another of my favorites is egregious use of caching, because traversing a DAG can result in the same decision being made four or five times, and the 'obvious' solution is to just add caches and/or promises to fix the problem. As near as I can tell, this dates back to a time when accumulating two copies of data into memory was considered a faux pas, and so we try to stream the data and work with it at the same time. We don't live there anymore, and because we don't live there anymore we are expected to handle bigger problems, like DAGs instead of lists or trees. These incremental solutions only work with streams and sometimes trees. They don't work with graphs. Critically, if the reason you're creating duplicate work is because you're subconsciously trying to conserve memory by acting while traversing, then adding caches completely sabotages that goal (and a number of others). If you build the plan first, then executing it is effectively dynamic programming. Or as you've pointed out, you can just not execute it at all. Plus the testing burden is so drastically reduced that I get super-frustrated having to have this conversation with people over and over again.
- sam0x17 5y agoIndeed. I would say that framework or even language-level support for putting things in "dry-run" mode is something sorely missed from many modern frameworks and languages, that old C libraries used to do.
- abrookewood 5y ago100% on the logging and dry run.
- ivanhoe 4y agoBeside doing this, I like to first just move files to another dir (keeping the relative path) instead of deleting them. It's basically like a DIY recycle bin. If both paths are on the same disk moving files is a fast operation - and if you discover a screw up, you can easily undo it. On the other hand if everything still looks fine after a few days, you just `rm -rf` that folder and purge the files.
- Too 4y agoA few assertions would have also stopped this. During buildup of the our_id list: assert (vimeoId not in our_ids). After creating the list: assert len(set(our_ids)) > 10000 and assert len(set(our_ids)) == len(our_ids) Before each final deletion: assert id not in hardcoded_list_of_golden_samples. Depending on the speed required you could hit the api again here as an extra check. But as always everything is obvious in hindsight. Even with the checks above, Plan+Apply is the safest approach.