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

deps: update exposed to v0.50.0 #8599

Merged
merged 2 commits into from
Apr 30, 2024
Merged

deps: update exposed to v0.50.0 #8599

merged 2 commits into from
Apr 30, 2024

Conversation

renovate[bot]
Copy link
Contributor

@renovate renovate bot commented Apr 29, 2024

Mend Renovate

This PR contains the following updates:

Package Change Age Adoption Passing Confidence
org.jetbrains.exposed:exposed-json 0.49.0 -> 0.50.0 age adoption passing confidence
org.jetbrains.exposed:exposed-jdbc 0.49.0 -> 0.50.0 age adoption passing confidence
org.jetbrains.exposed:exposed-java-time 0.49.0 -> 0.50.0 age adoption passing confidence
org.jetbrains.exposed:exposed-dao 0.49.0 -> 0.50.0 age adoption passing confidence
org.jetbrains.exposed:exposed-core 0.49.0 -> 0.50.0 age adoption passing confidence

Configuration

📅 Schedule: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 Automerge: Disabled by config. Please merge this manually once you are satisfied.

Rebasing: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 Ignore: Close this PR and you won't be reminded about these updates again.


  • If you want to rebase/retry this PR, check this box

This PR has been generated by Mend Renovate. View repository job log here.

@renovate renovate bot requested a review from a team as a code owner April 29, 2024 21:55
@renovate renovate bot added the dependencies Pull requests that update a dependency file label Apr 29, 2024
@sschuberth
Copy link
Member

@fviernau, @mnonnenmacher, while this Exposed release has no release notes yet, it apparently added some type safety features. This seems to conflict with some of the raw SQL statements used esp. in the helper-cli, like

val ids = database.transaction {
// language=PostgreSQL
"""
SELECT ${ScanResults.id.name}
FROM ${ScanResults.tableName}
WHERE (${ScanResults.scanResult.name} -> 'summary' -> 'licenses')::jsonb @>
'[{"license": "$license"}]'::jsonb
""".trimIndent().execAndMap {
it.getInt(ScanResults.id.name)
}
}
ScanResults.id inList ids

I quickly tried to convert this to the Exposed DSL, but failed. Could one of you assist here?

@fviernau
Copy link
Member

I quickly tried to convert this to the Exposed DSL, but failed. Could one of you assist here?

I'm taking a look...

@fviernau
Copy link
Member

fviernau commented Apr 30, 2024

Findings for DeleteCommand

All in all the command is heavily outdated at least in the following regards:

  1. The connectPostgresStorage() does not work anymore, because the key is no more postgresStorage but just postgres.
  2. The whole command operates on the legacy scan results table which does not exist anymore. Nowadays we have scan by provenance.

My guess of the original intended use cases, after looking into Git history:

  1. Delete scan result by package id and / or provenance. The commit does not specify the use case.
    However, I guess that with scan by provenance the use case would be best support without deleting
    the scan result, but deleting the provenance resolution entry.
  2. Delete scan result by license. This was supposed to be used after a detected license mapping changed.
    E.g. delete all scan results which became invalidated by changing the detected license mapping.

Proposal:

Just drop the entire delete command for now. @MarcelBochtler would that be fine with you (as you invented it) ?

@mnonnenmacher
Copy link
Member

Just drop the entire delete command for now. @MarcelBochtler would that be fine with you (as you invented it) ?

Marcel is not available for the whole week, but I assume he would be fine with that as we now use the ORT Server which has its own storage implementation where this command would not work anyway.

@sschuberth
Copy link
Member

I assume he would be fine

Ok, thanks both, then let me simply drop that command as part of this PR.

sschuberth added a commit that referenced this pull request Apr 30, 2024
…storage"

Remove the command to delete scan storage entries by reverting commit
64bf123 because the command is heavily outdated [1]. This prepares for an
Exposed update.

[1]: #8599 (comment)

Signed-off-by: Sebastian Schuberth <sebastian@doubleopen.org>
sschuberth and others added 2 commits April 30, 2024 12:05
…storage"

Remove the command to delete scan storage entries by reverting commit
64bf123 because the command is heavily outdated [1]. This prepares for an
Exposed update.

[1]: #8599 (comment)

Signed-off-by: Sebastian Schuberth <sebastian@doubleopen.org>
@sschuberth sschuberth enabled auto-merge (rebase) April 30, 2024 10:06
Copy link

codecov bot commented Apr 30, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 67.95%. Comparing base (17d14cd) to head (32642e4).
Report is 6 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #8599      +/-   ##
============================================
+ Coverage     67.70%   67.95%   +0.25%     
+ Complexity     1007     1005       -2     
============================================
  Files           246      244       -2     
  Lines          7924     7846      -78     
  Branches        883      876       -7     
============================================
- Hits           5365     5332      -33     
+ Misses         2176     2131      -45     
  Partials        383      383              
Flag Coverage Δ
funTest-non-docker 34.61% <100.00%> (-0.11%) ⬇️
test 37.93% <0.00%> (+0.37%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@sschuberth sschuberth disabled auto-merge April 30, 2024 11:28
@sschuberth
Copy link
Member

The NuGetFunTest failure is unrelated.

@sschuberth sschuberth merged commit 92d2be5 into main Apr 30, 2024
20 of 21 checks passed
@sschuberth sschuberth deleted the renovate/exposed branch April 30, 2024 11:28
sschuberth added a commit that referenced this pull request Apr 30, 2024
…storage"

Remove the command to delete scan storage entries by reverting commit
64bf123 because the command is heavily outdated [1]. This prepares for an
Exposed update.

[1]: #8599 (comment)

Signed-off-by: Sebastian Schuberth <sebastian@doubleopen.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants