This has all been tread over before -- I won't rehash it since it seems like we're going in circles.
The hot path is worth optimizing first, in my mind -- giving up perf there for stats which will be called much less doesn't make sense to me.
To each their own!
> 1st. Sorting on the web API is definitively wrong. This is especially true in nodejs where the mantra is always non blocking code.
This assertion is so wide that it cannot be true. No matter what language you're in, you can absolutely do sorting in the API layer.
> 2nd. Pipeline should 100 percent be known. Atomic operations are 100 percent required across shared storage and it's expected to occur all the time. Database transactions are fundamental and not unexpected. Pipeline should be extremely common. This is not an obscure operation. I commented about it in my code in case the reviewer was someone like you who isn't familiar with how popular atomic database transactions are, but this operation is common it is not a obscure trick.
You don't need to pipeline if the operation is O(1) -- it's not a question of whether transactions are good/bad, I don't know why you're assuming people don't know about them.
It's the question of taking in unnecessary complexity here when there's an O(1) operation available. You're optimizing for an operation that is not on the hot path. The reason you have to do the atomic operation is because you're using an access pattern that requires it.
I know why and how you used an atomic transaction -- I disagree on whether it was necessary.
> That'd be stupid in my opinion. 302 redirect is something I expected. Maybe I should have stated that explicitly in the docs. > > What? In the specs all examples have trailing slashes. All of them.
This is correct, I read wrong and for that I apologize, they do have all trailing slashes. I was thinking of "/api/".
> Unlikely. Even as a lead it's better to cater to the majority opinion of the team. 99 percent of the time you never have full control. > > Developers and leads typically evolve to fit into the overall team culture while inserting a bit of their own opinions. >
I referred to the context of the project -- you did* have full control.
If you were thinking of it in a team context, you should have asked more clarifying questions and found out more of what they wanted or would look for, similar to a real team context.
> No there isn't. You have to spin up redis. Flask documentation doesn't talk about that. Integration testing involves code that controls docker-compose. It's a bunch of hacky scripts with external process calls that are needed to get integration tests working. > > It's not rocket science but not within the scope of a take-home. Additionally hacking with docker compose is not sustainable or future proof, eventually you will hit infra that can't be replicated with docker. > > I will probably do it next time just to cater to people who share your viewpoints.
Again, this is a matter of what people think is hard and what isn't. Integration testing does not require that you spin up docker compose -- I literally linked to a libraries that do this in a reusable (importable) way w/ docker.
You said you don't do this kind of testing regularly, so it is understandable that it's not easy for you.
If docker was the problem, you can open subprocesses and assume that redis is installed in the test environment (i.e. your laptop, or CI environment), or take a binary to the redis binary path from ENV.
You can even remove the burden of running redis from the test suite and just make sure it's running manually or in test suite invocation/setup. There are many options, and it's just not that hard in 2023.
Don't know what (if anything) turned them off, so can't say this matters -- but it would matter to me, at the very least it would be a large plus for people who found the time to implement it.
> Lol. The prompt was write it in four hours ( no time) and they aren't paying me for it ( no money).
This is true, you did not have much time and they weren't paying you so I guess the situations are similar.