Skip to content

✨ adds some extra text context to sql statement errors#953

Merged
openshift-merge-robot merged 1 commit into
operator-framework:masterfrom
camilamacedo86:add-context-only
May 24, 2022
Merged

✨ adds some extra text context to sql statement errors#953
openshift-merge-robot merged 1 commit into
operator-framework:masterfrom
camilamacedo86:add-context-only

Conversation

@camilamacedo86

@camilamacedo86 camilamacedo86 commented May 16, 2022

Copy link
Copy Markdown
Contributor

Description

  • Add some extra context to the errors, mainly to retrieve the data (SQL)

Motivation

Tested locally

Screenshot 2022-05-18 at 14 10 44

Closes #952 because, via OPM, we can only provide extra info to clarify the scenario/why the error occurs.

@openshift-ci openshift-ci Bot requested review from anik120 and gallettilance May 16, 2022 19:42
@camilamacedo86 camilamacedo86 changed the title ✨ add context for some errors which can be faced to load the packages WIP: ✨ add context for some errors which can be faced to load the packages May 16, 2022
@openshift-ci openshift-ci Bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 16, 2022
@camilamacedo86 camilamacedo86 changed the title WIP: ✨ add context for some errors which can be faced to load the packages WIP: ✨ add context for sql errors and to obtain data May 17, 2022
@camilamacedo86 camilamacedo86 changed the title WIP: ✨ add context for sql errors and to obtain data WIP: ✨ add context for some sql errors and to obtain data May 17, 2022
@camilamacedo86 camilamacedo86 changed the title WIP: ✨ add context for some sql errors and to obtain data ✨ add context for some sql errors and to obtain data May 17, 2022
@openshift-ci openshift-ci Bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 17, 2022
@camilamacedo86 camilamacedo86 changed the title ✨ add context for some sql errors and to obtain data ✨ add context for some sql statements May 17, 2022
@codecov

codecov Bot commented May 17, 2022

Copy link
Copy Markdown

Codecov Report

Merging #953 (6b9b254) into master (fd85a98) will decrease coverage by 0.06%.
The diff coverage is 11.11%.

❗ Current head 6b9b254 differs from pull request most recent head 379105b. Consider uploading reports for the commit 379105b to get more accurate results

@@            Coverage Diff             @@
##           master     #953      +/-   ##
==========================================
- Coverage   52.42%   52.35%   -0.07%     
==========================================
  Files         103      103              
  Lines        9228     9238      +10     
==========================================
- Hits         4838     4837       -1     
- Misses       3468     3479      +11     
  Partials      922      922              
Impacted Files Coverage Δ
pkg/registry/populator.go 63.15% <0.00%> (ø)
pkg/sqlite/loadprocs.go 28.00% <0.00%> (-5.34%) ⬇️
pkg/sqlite/load.go 46.22% <14.70%> (-0.07%) ⬇️
pkg/registry/query.go 60.41% <0.00%> (-0.53%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fd85a98...379105b. Read the comment docs.

@camilamacedo86 camilamacedo86 changed the title ✨ add context for some sql statements 🐛 : check if channel entry exist before remove and adds some extra text context to sql statement errors May 17, 2022
Comment thread pkg/sqlite/load.go Outdated
Comment thread pkg/sqlite/load.go Outdated
Comment thread pkg/sqlite/load.go Outdated
@camilamacedo86 camilamacedo86 requested a review from grokspawn May 18, 2022 09:49
Comment thread pkg/sqlite/load.go Outdated
@grokspawn

Copy link
Copy Markdown
Contributor

/lgtm

@openshift-ci openshift-ci Bot removed the lgtm Indicates that a PR is ready to be merged. label May 19, 2022
Signed-off-by: Camila Macedo <cmacedo@redhat.com>
@camilamacedo86 camilamacedo86 changed the title 🐛 : check if channel entry exist before remove and adds some extra text context to sql statement errors ✨ adds some extra text context to sql statement errors May 19, 2022

@ankitathomas ankitathomas left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label May 19, 2022

@dinhxuanvu dinhxuanvu left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looking good. Just one question.

Comment thread pkg/registry/populator.go

@dinhxuanvu dinhxuanvu left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

/approve

@openshift-ci

openshift-ci Bot commented May 24, 2022

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ankitathomas, camilamacedo86, dinhxuanvu, grokspawn

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

The pull request process is described here

Details 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

@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 24, 2022
@openshift-merge-robot openshift-merge-robot merged commit dd41163 into operator-framework:master May 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

error loading bundle into db: FOREIGN KEY constraint failed

5 participants