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

Restructure redis queue entries #116

Open
ChandlerSwift opened this issue Jan 26, 2021 · 4 comments
Open

Restructure redis queue entries #116

ChandlerSwift opened this issue Jan 26, 2021 · 4 comments

Comments

@ChandlerSwift
Copy link
Member

Currently our redis queue entries are of the form {team_id}-{attack_id}. This has worked, but it's pretty inflexible, and that's going to cause some problems for #103 in particular. Rather than the custom encoding/decoding logic we have, we should replace this with something extensible. JSON would be an easy possibility:

{
    "team_id": 1,
    "attack_id": 1,
    "potential_extra_data": "goes_here",
    ...
}
@ChandlerSwift
Copy link
Member Author

In particular, this could be helpful for adding specifying what git branch to score against, likely as part of #103.

@derpferd
Copy link
Contributor

The reason that the redis queue entries weren't extensible was to make sure that duplicate requests would be deduped when added to the queue. JSON serialization doesn't guarantee that equal objects result in equal string values. Just something to think about.

ChandlerSwift added a commit that referenced this issue Feb 1, 2021
This should enable us to pass extra data between the worker and the scorer.
@ChandlerSwift
Copy link
Member Author

@derpferd, from my testing, it seems that if no extra data is passed (and with json.dumps({}, sort_keys=True) to prevent random ordering of keys) we do get equal string values. Of course, different versions of Python could treat whitespace differently, but in that case we just end up doing one unnecessary score run. That said, is there something I'm missing here?

@derpferd
Copy link
Contributor

derpferd commented Feb 1, 2021

Yes, I think that would work. I am sure you can also explicitly say how to handle whitespace as well.

Another thing to keep in mind is the potential_extra_data containing some data that could reduce the number of possible dedups to zero. An example would be adding a timestamp.

With these two things in mind it shouldn't be a problem.

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

No branches or pull requests

2 participants