Skip to content

[int] Parameterise SQL in AccountingDB#8669

Merged
fstagni merged 1 commit into
DIRACGrid:integrationfrom
sfayer:fix_acctsql
Jul 8, 2026
Merged

[int] Parameterise SQL in AccountingDB#8669
fstagni merged 1 commit into
DIRACGrid:integrationfrom
sfayer:fix_acctsql

Conversation

@sfayer

@sfayer sfayer commented Jul 5, 2026

Copy link
Copy Markdown
Member

Part of #8547.

This is probably the one I'm most worried about in the set as the AccountingSystem does a lot of custom SQL: I'm not convinced the test coverage here is very complete. Do you have any suggestion on how we can test it further? (Perhaps we could put it on certification or something like that?)

BEGINRELEASENOTES
*Accounting
FIX: Parameterise SQL in AccountingDB
ENDRELEASENOTES

@sfayer sfayer requested review from atsareg and fstagni as code owners July 5, 2026 21:56
@sfayer sfayer marked this pull request as draft July 6, 2026 08:24
@fstagni

fstagni commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

I'm not convinced the test coverage here is very complete.

You are over-estimating the test coverage for everything else... :)

Anyway, for the moment some tests are failing.

Do you have any suggestion on how we can test it further? (Perhaps we could put it on certification or something like that?)

That's always an option.

@sfayer sfayer force-pushed the fix_acctsql branch 3 times, most recently from a90e6b6 to c21067c Compare July 6, 2026 12:25
@sfayer

sfayer commented Jul 6, 2026

Copy link
Copy Markdown
Member Author

I've fixed the tests: I looked with @marianne013 at testing on certification but we decided that was probably excessive in this instance.

I guess you want to queue this one after after #8677 ? I'll rebase it once that one is complete...

Regards,
Simon

@sfayer sfayer marked this pull request as ready for review July 6, 2026 16:58
@fstagni

fstagni commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

I guess you want to queue this one after after #8677 ? I'll rebase it once that one is complete...

Yes, that would simplify things. Mind reviewing that one?

@sfayer

sfayer commented Jul 7, 2026

Copy link
Copy Markdown
Member Author

I've rebased this one now, so it's up to date with the recent AccountingDB changes (it's already behind the branch again due to other merges in the last 30 minutes, but shouldn't conflict).

@fstagni fstagni merged commit 6fb2599 into DIRACGrid:integration Jul 8, 2026
23 checks passed
@DIRACGridBot DIRACGridBot added the sweep:ignore Prevent sweeping from being ran for this PR label Jul 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

sweep:ignore Prevent sweeping from being ran for this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants