Base RunId comparison and hash on all details #271
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is in preparation for the support of #257, which will add machine and denoise configuration support. Though, it’s also useful without, because it allows us to distinguish RunIds that differ in their environment variables, for instance. Or more generally, RunIds that differ based on any property that is not strictly part of the command line, which is was previously used to establish equality. This as a consequence means, we have a much weaker ability to determine equality of RunIds than before, but I think that’s fine and less surprising/buggy. The main changes are a proper implementation of the __eq__, __lt__, and __hash__ methods, as well as the serialization of RunIds into the data file, and deserialization when loading a data file. This change includes some minor refactorings, because it is split from a too huge and unmanagable patch. Minor refactorings: - configurator: extract config validation and assembling of run details - executor: use exe name and suite name directly in indicate_build - executor/BuildCommand: location is in BuildCommand only for equality, but semantically, we should use the location/path of the suite/executor when processing the build command. That’s now made explicit, and asserted for correctness. - BuildCommand is now storing only the command, since the location is used from suite/exe Signed-off-by: Stefan Marr <[email protected]>
Instead of failing, replace unsupported bytes with a replacement marker. This avoids failing with an exception and allows to proceed to get some data from the output. Signed-off-by: Stefan Marr <[email protected]>
This is possible, because the equality/comparison is now including everything. Signed-off-by: Stefan Marr <[email protected]>
Signed-off-by: Stefan Marr <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR is in preparation for the support of #257, which will add machine and denoise configuration support.
Though, it’s also useful without, because it allows us to distinguish RunIds that differ in their environment variables, for instance. Or more generally, RunIds that differ based on any property that is not strictly part of the command line, which is was previously used to establish equality.
This as a consequence means, we have a much weaker ability to determine equality of RunIds than before, but I think that’s fine and less surprising/buggy. It consequently undos some of #4.
On the other hand, it should be everything needed to resolve #119.
The main changes are a proper implementation of the
__eq__,__lt__, and__hash__methods, as well as the serialization of RunIds into the data file, and deserialization when loading a data file.This change includes some minor refactorings, because it is split from a too huge and unmanageable patch.
Minor refactorings:
TODO: