-
Notifications
You must be signed in to change notification settings - Fork 11
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
display build URL for changepoint #15
display build URL for changepoint #15
Conversation
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.
LGTM
7ba5c60
to
3090000
Compare
@shashank-boyapally we need a rebase of this PR |
Signed-off-by: Shashank Reddy Boyapally <sboyapal@redhat.com>
Signed-off-by: Shashank Reddy Boyapally <sboyapal@redhat.com>
Signed-off-by: Shashank Reddy Boyapally <sboyapal@redhat.com>
Signed-off-by: Shashank Reddy Boyapally <sboyapal@redhat.com>
1cc6b70
to
972cf55
Compare
rebased this, with changes to use our new internal instance. |
orion.py
Outdated
if len(uuids) == 0: | ||
logging.info("No UUID present for given metadata") | ||
sys.exit() | ||
else: | ||
uuids = [uuid for uuid in re.split(' |,',baseline) if uuid] | ||
uuids.append(uuid) | ||
if metadata["benchmark.keyword"] == "k8s-netperf" : | ||
if metadata["benchmark.keyword"] == "ospst-k8s-netperf" : |
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.
I think we are going to need both ospst-k8s-netperf and k8s-netperf to work depending on which ES_SERVER we are looking at. Can we change the if statement to if "k8s-netperf" in metadata["benchmark.keyword"]
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.
@shashank-boyapally is work for allowing multiple ES_SERVERs part of a different PR?
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.
The discussion in the slack channel for allowing multiple ES_SERVERs in the same run is not yet developed, this just take cares for the internal instance changes we had.. will be developing that once I understand the further requirements more clearly.
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.
I still think the if "k8s-netperf" in metadata["benchmark.keyword"]
would be better because this update breaks the usage of the QE elastic search index completely. This change would allow both to still work
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.
Yeah, I'll keep it as the previous, but what index would we like to search is to be decided, as I believe in QE instance the index is k8s-netperf
and in the internal instance it is ospst-k8s-netperf
, should we include this index in the config file itself for better flexibility?
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.
yes I understand the index is different between QE instance and the internal instance. The code block I put in previous comments will help cover both instances. I think setting in the config will make it less flexible as we will have to have multiple files to pull the same type of data from each of the instances. That might need to be something addressed with the pr for setting multiple ES_SERVERS
Signed-off-by: Shashank Reddy Boyapally <sboyapal@redhat.com>
orion.py
Outdated
ids = uuids | ||
else: | ||
index = "ripsaw-kube-burner" | ||
index = "ospst-ripsaw-kube-burner*" |
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.
should add the same starting * as k8s-netperf and ingress-perf here
*ripsaw-kube-burner*
Signed-off-by: Shashank Reddy Boyapally <sboyapal@redhat.com>
Signed-off-by: Shashank Reddy Boyapally <sboyapal@redhat.com>
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.
lgtm
lgtm! Thanks! |
Type of change
Description
Since it will be easier to check buildUrl from the console, this would be an addition to orion. I'm unable to add the hyperlinked text in this preview, but the terminal does show hyperlinked text. The reason moving to hyperlinked text is to keep the terminal readable and clean. If having the URL makes more sense, it can be updated
Related Tickets & Documents
Checklist before requesting a review
Testing