-
Notifications
You must be signed in to change notification settings - Fork 14
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
Convert prow-to-es functions to python #107
Convert prow-to-es functions to python #107
Conversation
core/opl/shovel.py
Outdated
to_path = f"{args.prow_data_file}" | ||
if not os.path.isfile(to_path): | ||
print(f"INFO: Downloading {from_url} ... ", end="") | ||
subprocess.run(["curl", "-Ssl", "-o", to_path, from_url], check=True) |
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.
Hello! Just use requests
here.
core/opl/shovel.py
Outdated
|
||
|
||
class pluginProw: | ||
def __init__(self, args): | ||
print("Hello from pluginProw init") | ||
print("Hello from pluginProw init 2") |
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.
Use logging....
instead of prints pls.
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.
Also these prints created by me can be removed, they were there just for demo purposes
core/opl/shovel.py
Outdated
|
||
|
||
class pluginOpenSearch: | ||
def __init__(self, args): | ||
print("Hello from pluginOpenSearch init") | ||
self.logger = logging.getLogger("opl.showel.pluginOpenSearch") | ||
|
||
def upload(self): | ||
def upload(self, args): |
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.
Store args
to serf.args
in __init__
.
core/opl/shovel.py
Outdated
@@ -58,15 +69,41 @@ def args(parser, group_actions): | |||
default="redhat-appstudio-load-test/artifacts/load-tests.json", | |||
help="Path to the artifact", | |||
) | |||
group.add_argument("--prow-data-file", help="prow data jdon file") |
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 there is no default=
, should there be required=true
?
core/opl/shovel.py
Outdated
|
||
|
||
class pluginHorreum: | ||
def __init__(self, args): | ||
print("Hello from pluginHorreum init") | ||
self.logger = logging.getLogger("opl.showel.pluginHorreum") | ||
|
||
def upload(self): | ||
print("Hello from pluginHorreum upload") | ||
def upload(self, args): |
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.
Again, save args
in __init__
core/opl/shovel.py
Outdated
def upload(self): | ||
print("Hello from pluginHorreum upload") | ||
def upload(self, args): | ||
if args.horreum_data_file is None: |
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.
Just add required=True
to add_argumet
calls for these options and you do not need to do this.
core/opl/shovel.py
Outdated
values = json.load(jsonFile) | ||
jsonFile.close() | ||
test_matcher = values[args.test_job_matcher] | ||
test_start = values["timestamp"] |
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.
Let's not rely on start/end time existing in the JSON, make user to provide it on command line. Same for $schema
.
core/opl/shovel.py
Outdated
) | ||
test_id = json.load(response.text)["id"] | ||
page = 0 | ||
while True: |
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.
Recently I have been given more straight forward way how to check for result presence in Horreum: https://github.com/openshift-pipelines/performance/blob/main/ci-scripts/prow-to-storage.sh#L119 Please use that instead.
core/opl/shovel.py
Outdated
raise Exception("End timestamp is needed to work with --opensearch-upload") | ||
else: | ||
json_data = json.dumps( | ||
{"query": {"match": {"endTimestamp": args.end_timestamp}}} |
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.
Make the field configurable via options. For some project, we might want to rely on different field.
core/opl/shovel.py
Outdated
if test_start == "None": | ||
test_start = values["startTimestamp"] | ||
test_end = values["endTimestamp"] | ||
if ( |
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 start/end times are in options with required=True
, argparse will take care about this check. You can even enforce in argparse option setup provided value to be date: https://stackoverflow.com/questions/21437258/how-do-i-parse-a-date-as-an-argument-with-argparse
core/opl/shovel.py
Outdated
jsonFile = open(args.horreum_data_file, "r") | ||
values = json.load(jsonFile) | ||
jsonFile.close() | ||
test_start = values["timestamp"] |
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.
Make the user to provide these.
core/opl/shovel.py
Outdated
json.dump(annotation_data, file) | ||
|
||
# Send a POST request to the API and retrieve the result using curl and jq | ||
curl_command = f"curl https://{args.horreum_host}/api/changes/annotations -s -H 'content-type: application/json' -d @/tmp/annotationQuery.json | jq -r ." |
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.
Use Python requests
library here. With that, you do not need to create a file.
core/opl/shovel.py
Outdated
group.add_argument("--test-owner", default="rhtap-perf-test-team") | ||
group.add_argument("--test-access", default="PUBLIC") | ||
group.add_argument( | ||
"--id-array", help="Variable id array we wish to get timeseries data for" |
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 it is actually for? Could you point me for some docs somewhere? Maybe add more useful help text, with this I feel bit lost :) Can the user maybe just provide coma separated list of IDs instead of JSON?
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.
Thank you!
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.
Thank you!
No description provided.