-
Notifications
You must be signed in to change notification settings - Fork 10
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
Previous version comparison and baseline updates #86
base: main
Are you sure you want to change the base?
Conversation
d4c55bb
to
2bc4fa6
Compare
c169e04
to
89b4aec
Compare
See this test run: https://jenkins-csb-openshift-qe-mastern.dno.corp.redhat.com/job/scale-ci/job/paige-e2e-multibranch/job/orion/110/console The main updates that I was trying to do with refactoring is that when the uuid and baseline are defined we don't want to do elastic calls to check if the run was success. If we specify the uuid/baseline uuid we don't want to filter them out and just use them as the base to run comparisons |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice addition @paigerube14 !
I am debugging this issue :
running |
elif not uuids: | ||
runs = match.getResults("", uuids, fingerprint_index, {}) | ||
# get metadata of one run | ||
metadata = get_metadata_with_uuid(options["uuid"], match) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if uuid
is not specified in the options? We have a null check for this in the below code block but not here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if baseline is specified uuid has to be specified. Its required from the variable options
if len(last_version_run) > 0: | ||
# get latest uuid as the "uuid" to compare against | ||
last_uuid_run = last_version_run[-1] | ||
runs.append(last_uuid_run) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are we only appending one uuid
from the current version? Do we plan to use this only in CMR? Can't we do all runs from previous version vs current version comparison?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any of the algorithms should work as this is just gathering the UUIDs before it performs hunter or cmr, etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the future, we can maybe add in an option of how many current UUIDs to add when comparing with the previous UUID data. For now I just put one so that we could compare and see if any large changes in data points
a75214d
to
5501712
Compare
rh-pre-commit.version: 2.2.0 rh-pre-commit.check-secrets: ENABLED Signed-off-by: Paige Patton <[email protected]>
@jtaleric I am seeing that message every time I run no matter if I run with previous or not
|
pkg/utils.py
Outdated
runs = match.get_uuid_by_metadata(metadata, fingerprint_index, lookback_date=start_timestamp, lookback_size=options['lookback_size']) | ||
if len(last_version_run) > 0: | ||
# get latest uuid as the "uuid" to compare against | ||
last_uuid_run = last_version_run[-1] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thinking here might be better to get the last (0) of the specified version. If not, when giving the 15 day lookback it'll select the oldest UUID. Thoughts?
Type of change
Description
In this PR I have added a way for us to compare data on with the most recent run (or specified uuid) to a previous release
I also updated the code that if a baseline uuid is given we do not need to call to get the metadata specified as a user would expect only the data of the uuid and baseline uuid to be defined in their output
Related Tickets & Documents
Checklist before requesting a review
Testing
orion cmd --config configs/small-scale-cluster-density.yaml --previous-version
A change that might still be needed past this is adding in a column into the base information of the run to show the version