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

entries for staging and production #21

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

entries for staging and production #21

wants to merge 8 commits into from

Conversation

burnout87
Copy link

No description provided.

@burnout87 burnout87 linked an issue Feb 27, 2024 that may be closed by this pull request
@burnout87
Copy link
Author

I developed the following:

  • the inspect cli from the oda_api is called using the group-by-job arg and specifying the dispatcher url (as an env_var)
  • the generated dispatcher-state.json file is inspected in order to build a list of job_id(s) for which the request has not been completed (request_completed: false)

Something else should be inspected?
Do we want to save those job_id(s) in some specific location?

@volodymyrss volodymyrss self-assigned this Mar 4, 2024
@volodymyrss volodymyrss assigned burnout87 and unassigned volodymyrss Mar 11, 2024
@volodymyrss volodymyrss self-requested a review March 11, 2024 11:00
@@ -138,6 +138,42 @@ spec:
readOnly: true
{{- end }}

- name: {{ .Chart.Name }}-job-scanner-staging
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

deployment and staging are two deployments of the chart, you do not need both inside the chart.

- |
unset json

dispatcher_state=$(<dispatcher-state.json)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why set this variable?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not needed indeed


- name: {{ .Chart.Name }}-job-scanner-production
image: "{{ .Values.image.repository }}:{{ .Values.image.tag }}"
command: ["oda_api"]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better set the command to bash and write bash script as the second arg

job_id=`echo "$i" | jq -r '.job_id'`;
request_completed=`echo "$i" | jq '.job_status_data[].request_completed'`;
if [[ $request_completed != *"true"* ]]; then
uncompleted_job_ids+=($job_id);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and then what happens to uncompleted_job_ids variable?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understood we wanted to potentially re-submit those uncompleted jobs...otherwise what do we do with them?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, so propose a command to resubmit?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

now it extracts, when possible, instrument product_query and job_id, and resubmit the same request

Copy link
Member

@volodymyrss volodymyrss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Few questions

@burnout87 burnout87 requested a review from volodymyrss March 13, 2024 13:57
@volodymyrss volodymyrss requested a review from dsavchenko August 27, 2024 12:34
command: ["bash"]
args:
- |
oda-api -u DISPATCHER_URL inspect --group-by-job;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think what would be good to add is some kind of failsafe, i.e. when this whole script fails the container should not crash.
Else it can not be deployed and it's annoying. And for an optional feature like this it should get deployed anyway.

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

Successfully merging this pull request may close these issues.

add rescan of job status
2 participants