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.

Sign up for GitHub

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

Jump to bottom

Internal trig advise user api #616

Closed
wants to merge 1 commit into from
Closed

Internal trig advise user api #616

wants to merge 1 commit into from

Conversation

KPostOffice
Copy link
Member

Related Issues and Dependencies

Make internal trigger advise requests from investigator because Kebechet no longer sends advise requests unless specifically requested

This introduces a breaking change

  • Yes
  • No

This Pull Request implements

… Explain your changes.

Description

@sesheta sesheta added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jan 25, 2022
@sesheta sesheta requested review from fridex and harshad16 January 25, 2022 22:29
@KPostOffice KPostOffice force-pushed the internal_trig_advise_user_api branch from 0ad4c8c to 2c90bc8 Compare January 25, 2022 22:30
@sesheta sesheta added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jan 25, 2022
@KPostOffice
Copy link
Member Author

Fixes: #622

@KPostOffice KPostOffice force-pushed the internal_trig_advise_user_api branch from 2c90bc8 to ebb5b1f Compare February 7, 2022 19:17
@sesheta sesheta added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Feb 7, 2022
@KPostOffice KPostOffice force-pushed the internal_trig_advise_user_api branch 2 times, most recently from 8d1d9ac to 1660395 Compare March 8, 2022 22:33
@sesheta sesheta added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Mar 8, 2022
@KPostOffice KPostOffice force-pushed the internal_trig_advise_user_api branch 3 times, most recently from 738a28a to feeb93e Compare March 11, 2022 20:00
@KPostOffice KPostOffice marked this pull request as ready for review March 11, 2022 20:00
@sesheta sesheta removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 11, 2022
@KPostOffice KPostOffice force-pushed the internal_trig_advise_user_api branch 3 times, most recently from 0367cb6 to 39800e2 Compare March 11, 2022 21:55
harshad16
harshad16 reviewed Mar 14, 2022
View reviewed changes
Copy link
Member

@harshad16 harshad16 left a comment

Choose a reason for hiding this comment

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

lgtm
is this tested ? will it be possible to be tested in test cluster?

@harshad16
Copy link
Member

/deploy

@KPostOffice
Copy link
Member Author

@harshad16 Yeah, I tested this locally. Monitoring behavior in the test cluster is definitely a good idea as these are pretty large changes.

@codificat codificat mentioned this pull request Mar 17, 2022
Merged
@codificat
Copy link
Member

Unsure why the mypy check is failing, but hoping that #635 might fix it

@harshad16
Copy link
Member

/deploy

@harshad16
Copy link
Member

Yeah, I tested this locally. Monitoring behavior in the test cluster is definitely a good idea as these are pretty large changes.

ack, will get this in test

@sesheta
Copy link
Member

sesheta commented Jun 15, 2022

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

/lifecycle stale

@sesheta sesheta added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jun 15, 2022
@KPostOffice
Copy link
Member Author

/remove-lifecycle stale

@sesheta sesheta removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jun 21, 2022
@goern
Copy link
Member

goern commented Jun 22, 2022

what is the status of this?

@KPostOffice
Copy link
Member Author

what is the status of this?

Need to wrap my head back around it. I think the main thing it is missing is a couple of environment variables in the investigator deployment. I'll have a look at this tomorrow.

@KPostOffice KPostOffice force-pushed the internal_trig_advise_user_api branch from 39800e2 to b8aae11 Compare June 23, 2022 19:51
@sesheta sesheta added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jun 23, 2022
@KPostOffice KPostOffice mentioned this pull request Jun 23, 2022
Merged
@KPostOffice KPostOffice force-pushed the internal_trig_advise_user_api branch from b8aae11 to 2ebb197 Compare June 28, 2022 19:27
codificat
codificat requested changes Jun 30, 2022
View reviewed changes
Copy link
Member

@codificat codificat left a comment

Choose a reason for hiding this comment

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

There are some unresolved conflicts that slipped in...

thoth/investigator/common.py Outdated
@@ -103,6 +108,7 @@ async def wait_for_limit(openshift: OpenShift, workflow_namespace: str):
_LOGGER.debug("Current number pending = %d", total_pending)


<<<<<<< HEAD
Copy link
Member

Choose a reason for hiding this comment

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

unresolved conflict

thoth/investigator/hash_mismatch/investigate_hash_mismatch.py Outdated
@@ -65,6 +73,7 @@ async def parse_hash_mismatch(mismatch: Dict[str, Any], openshift: OpenShift, gr
)

if Configuration.THOTH_INVESTIGATOR_SCHEDULE_KEBECHET_ADMIN:
<<<<<<< HEAD
Copy link
Member

Choose a reason for hiding this comment

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

unresolved conflict

thoth/investigator/missing_package/investigate_missing_package.py Outdated
@@ -55,6 +59,7 @@ async def parse_missing_package(package: Dict[str, Any], openshift: OpenShift, g
)

if Configuration.THOTH_INVESTIGATOR_SCHEDULE_KEBECHET_ADMIN:
<<<<<<< HEAD
Copy link
Member

Choose a reason for hiding this comment

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

unresolved conflict

@sesheta
Copy link
Member

sesheta commented Jun 30, 2022

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please ask for approval from kpostoffice after the PR has been reviewed.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@KPostOffice KPostOffice force-pushed the internal_trig_advise_user_api branch from 2ebb197 to 961cee4 Compare July 7, 2022 15:05
@sesheta sesheta added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jul 7, 2022
@KPostOffice KPostOffice force-pushed the internal_trig_advise_user_api branch from 961cee4 to cd9097e Compare July 7, 2022 20:25
@KPostOffice KPostOffice force-pushed the internal_trig_advise_user_api branch from cd9097e to d5df8bb Compare July 7, 2022 20:30
@sesheta
Copy link
Member

sesheta commented Jul 7, 2022

@KPostOffice: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
aicoe-ci/prow/mypy d5df8bb link true /test thoth-mypy-py38

Full PR test history. Your PR dashboard. Please help us and open an issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@KPostOffice KPostOffice marked this pull request as draft February 9, 2023 15:11
@sesheta sesheta added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@harshad16 harshad16 harshad16 left review comments

@codificat codificat codificat requested changes

@fridex fridex Awaiting requested review from fridex

Assignees
No one assigned
Labels
do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@KPostOffice @harshad16 @codificat @sesheta @goern