4 ms·

There's some nitpicky bits (like how "utils.py contains two utility functions" is then followed by _three_ bullet points :P ) but one big one that jumped out wa

There's some nitpicky bits (like how "utils.py contains two utility functions" is then followed by _three_ bullet points :P ) but one big one that jumped out was using "zrank" to get the new score. That's not going to return the right value most of the time - for instance, in the example from the docs at https://redis.io/docs/data-types/sorted-sets/ https://redis.io/docs/data-types/sorted-sets/ has this: zrank hackers "Anita Borg" is 4 but "Anita Borg"'s score is 1949. The operation you likely wanted was "zscore". It would be straightforward to check this, either manually or in an automated test - simulate hitting "/api/abc/" once and "/api/xyz/" three times. The result of 3 of the 4 requests will be wrong: for /api/abc/: will have count=0 because /api/abc/ is the only entry in the sorted set first one for /api/xyz/ will have count=1 because /api/abc/ sorts before /api/xyz/ and they both have a score of 1 second one for /api/xyz/ will have count=1 because /api/xyz/ has a higher score than /api/abc/ third one for /api/xyz/ will have count=1 because that's still true Now, it's true that the assignment didn't say anything about what the service should return: you might have done better on this assignment by returning HTTP 204 and an empty body. IMO when a "super senior / staff engineer" chooses to write code, they should write WORKING CODE and verify that that's true.

formulathree 3y ago
>IMO when a "super senior / staff engineer" chooses to write code, they should write WORKING CODE and verify that that's true. You can spin the code up with "docker-compose up --build" You and everyone else can then verify your statements on my project by running this: docker exec -it flask poetry run http localhost:5000/api/abc/ docker exec -it flask poetry run http localhost:5000/api/xyz/ docker exec -it flask poetry run http localhost:5000/api/xyz/ docker exec -it flask poetry run http localhost:5000/api/xyz/ (By the way the response on these calls actually returns the current count of the specific route for easier testing. You can see whether your hypothesis is real by watching the results unfold on each request.) Then run this: docker exec -it flask poetry run http localhost:5000/stats/ and the results are: [ { "count": 3, "path": "xyz" }, { "count": 1, "path": "abc" } ] Which is correct. Which means You are wrong sir. You did not test my code. Literally, I just ran this in my code, you can try it if you have the time. I mean you're not the only person to make wrong assumptions on this thread. I'm getting the sense that reviewers just take a quick look and make rash judgements and then throw it into the trash. I'm betting it's not too far off from what the reviewer did for me. You didn't run the code, which is fine, it's not expected of you. But shockingly, I think it's quite likely the evaluator at the company didn't run my code either and that is not expected or appropriate imo. I think it's safe to say that This thread in general and your response included is becoming an accurate representation of typical shenanigans that happen for take home projects. (Your mistake here fine, not a big deal, you're not evaluating me just giving me advice so getting things wrong is understandable in your case)
- ihumanable 3y ago
  So you might have an answer as to why you got a poor evaluation. Multiple people have had trouble following code, and for good reason, what's going on here? # - pipeline insures that the zincrby and zrank call happen atomically and prevents race conditions. # - zincrby increments the score on the string, zrank fetches the new score pipeline = redis.pipeline() pipeline.zincrby(URL_PATH_COUNT_DATA_STRUCTURE, 1, path) pipeline.zrank(URL_PATH_COUNT_DATA_STRUCTURE, path, ) data = await pipeline.execute() # returns the path and current count that was just made. return {"path": path, "count": int(data[0])} So you make a pipeline so that the zincrby and zrank can be atomic. But the zrank seems to be completely pointless, it does not, as the comment suggests, "fetches the new score." The reason the counts are correct is because you are pulling the first reply out of the pipeline which is the result of zincrby, which is the new score. So if I were evaluating your code I would wonder - Why is there a pipeline here at all? - Why is it zranking? - Did the comment go stale and the candidate just has poor attention to detail? - Is the comment the candidate telling me that they don't know how zrank works and they don't know how pipeline works? - Why is the comment lying to me? - Does this super senior engineer think "insure" and "ensure" are the same word? The above code seems to my reading to be identical to score = await redis.zincrby(URL_PATH_COUNT_DATA_STRUCTURE, 1, path) return {"path": path, "count" int(score)}
  - formulathree 3y ago
    Yes this is probably it. The biggest flaw in the code. I will note, only one person pointed this out in the entire thread and that's you. If there's any raw legitimate claim to a rejection it's this. Literally everyone else commented on a bunch of superficial things and no one hit on the extra zrank here. It's not even about not being able to follow the code. I think very few people even remember specific redis commands or even bothered to follow the logic.
formulathree 3y ago
>There's some nitpicky bits (like how "utils.py contains two utility functions" is then followed by _three_ bullet points :P ) There are three functions: https://github.com/anonanonme/takehome-sample/blob/master/utils.py https://github.com/anonanonme/takehome-sample/blob/master/ut... I don't judge people for these types of mistakes either. It's all nits. Especially in an HN comment. But someone evaluating a job candidate, in my opinion, needs to step up his game and I'm thinking a lot of these people are treating it the same way you're treating it.

formulathree 3y ago

>IMO when a "super senior / staff engineer" chooses to write code, they should write WORKING CODE and verify that that's true. You can spin the code up with "docker-compose up --build" You and everyone else can then verify your statements on my project by running this: docker exec -it flask poetry run http localhost:5000/api/abc/ docker exec -it flask poetry run http localhost:5000/api/xyz/ docker exec -it flask poetry run http localhost:5000/api/xyz/ docker exec -it flask poetry run http localhost:5000/api/xyz/ (By the way the response on these calls actually returns the current count of the specific route for easier testing. You can see whether your hypothesis is real by watching the results unfold on each request.) Then run this: docker exec -it flask poetry run http localhost:5000/stats/ and the results are: [ { "count": 3, "path": "xyz" }, { "count": 1, "path": "abc" } ] Which is correct. Which means You are wrong sir. You did not test my code. Literally, I just ran this in my code, you can try it if you have the time. I mean you're not the only person to make wrong assumptions on this thread. I'm getting the sense that reviewers just take a quick look and make rash judgements and then throw it into the trash. I'm betting it's not too far off from what the reviewer did for me. You didn't run the code, which is fine, it's not expected of you. But shockingly, I think it's quite likely the evaluator at the company didn't run my code either and that is not expected or appropriate imo. I think it's safe to say that This thread in general and your response included is becoming an accurate representation of typical shenanigans that happen for take home projects. (Your mistake here fine, not a big deal, you're not evaluating me just giving me advice so getting things wrong is understandable in your case)

ihumanable 3y ago
So you might have an answer as to why you got a poor evaluation. Multiple people have had trouble following code, and for good reason, what's going on here? # - pipeline insures that the zincrby and zrank call happen atomically and prevents race conditions. # - zincrby increments the score on the string, zrank fetches the new score pipeline = redis.pipeline() pipeline.zincrby(URL_PATH_COUNT_DATA_STRUCTURE, 1, path) pipeline.zrank(URL_PATH_COUNT_DATA_STRUCTURE, path, ) data = await pipeline.execute() # returns the path and current count that was just made. return {"path": path, "count": int(data[0])} So you make a pipeline so that the zincrby and zrank can be atomic. But the zrank seems to be completely pointless, it does not, as the comment suggests, "fetches the new score." The reason the counts are correct is because you are pulling the first reply out of the pipeline which is the result of zincrby, which is the new score. So if I were evaluating your code I would wonder - Why is there a pipeline here at all? - Why is it zranking? - Did the comment go stale and the candidate just has poor attention to detail? - Is the comment the candidate telling me that they don't know how zrank works and they don't know how pipeline works? - Why is the comment lying to me? - Does this super senior engineer think "insure" and "ensure" are the same word? The above code seems to my reading to be identical to score = await redis.zincrby(URL_PATH_COUNT_DATA_STRUCTURE, 1, path) return {"path": path, "count" int(score)}
- formulathree 3y ago
  Yes this is probably it. The biggest flaw in the code. I will note, only one person pointed this out in the entire thread and that's you. If there's any raw legitimate claim to a rejection it's this. Literally everyone else commented on a bunch of superficial things and no one hit on the extra zrank here. It's not even about not being able to follow the code. I think very few people even remember specific redis commands or even bothered to follow the logic.

formulathree 3y ago

>There's some nitpicky bits (like how "utils.py contains two utility functions" is then followed by _three_ bullet points :P ) There are three functions: https://github.com/anonanonme/takehome-sample/blob/master/utils.py https://github.com/anonanonme/takehome-sample/blob/master/ut... I don't judge people for these types of mistakes either. It's all nits. Especially in an HN comment. But someone evaluating a job candidate, in my opinion, needs to step up his game and I'm thinking a lot of these people are treating it the same way you're treating it.