feat: Add SubjectResource entity and db migration #1048
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
4 tasks
elsand
changed the title
elsand
commented
Aug 30, 2024
...Digdir.Domain.Dialogporten.Janitor/Features/UpdateSubjectResources/UpdateSubjectResources.cs
Outdated
Show resolved
Hide resolved
<!--- Provide a general summary of your changes in the Title above -->
## Description
Adds:
- Dockerfile
- Entry in a new docker-compose for jobs
- Bicep definitions and extra properties for specifying schedule and
command
- Github action to deploy jobs
<!--- Describe your changes in detail -->
## Related Issue(s)
- #{issue number}
## Verification
- [ ] **Your** code builds clean without any errors or warnings
- [ ] Manual testing done (required)
- [ ] Relevant automated test added (if you find this hard, leave it and
we'll help out)
## Documentation
- [ ] Documentation is updated (either in `docs`-directory, Altinnpedia
or a separate linked PR in
[altinn-studio-docs.](https://github.com/Altinn/altinn-studio-docs), if
applicable)
Hvis dette godkjennes, fungerer det slik: 1. Hent sist oppdatert dato fra enten kommando input, eller den største UpdatedAt i SubjectResources tabellen, eller DateTimeOffset.MinValue (i den rekkefølgen). 2. Stream subjekt-ressurs-mappings batchvis fra ressursregisteret 3. Stapp det inn i DB via en merge statement 4. Repiter steg 2 og 3 til ressursregisteret ikke har noen flere mappings MERK - nå brukes ikke SubjectResourceLastUpdates tabellen ettersom siste oppdateringsdato hentes fra SubjectResources. Jeg tror dette blir korrekt. I tillegg gjør dette at vi ikke trenger å ta hensyn til server clock drift ettersom SubjectResources.UpdatedAt hentes og populeres direkte fra ressurs registeret. Altså vi bruker ikke DP sin tid, vi bruker ressurs registeret sin tid. Eneste er at når vi er "ferdig" med syncen ser jeg at ressurs register apiet fremdeles returnerer data, som sier til meg at de henter data fra OG MED "since". IMO mener jeg dette er feil. Det burde IMO kun vært fra, med et unntakt på likhet innenfor tie-breakeren. Dette er den samme problemstillingen vi har løst på vår paginering. Jeg har tatt bort transaksnonshåndteringen fordi vi nå bruker en sql merge statement for hele batchen. Dvs hver batch kan håndteres i hver sin transaksjon. Men her kom jeg på noe. Dersom det såpass mange subjekt-ressurs-mappinger innenfor samme timestamp at det strekker seg over flere batcher og det skjer en exception kan man havne i en litt uggen tilstand. Si for enkelhets skyld at det er 100 stk innenfor samme tidspunkt og vi batcher i sett på 10. Dvs at vi må ha 10x10 successfull batches for å traversere til et nytt tidspunkt. Men dersom vi failer på batch nr 5 vil vi med steg 1 ovenfor hente ut det samme tidspunktet for alle 100 stk og starte fra starten igjen (batch 1). Det er ikke et problem slik apiet er nå (fra **og med** since), men dersom apiet endrer seg til kun fra since blir det et issue. Vi kan løse det med å generere vårt eget token som sendes inn til endepunktet for tie-breakeren, men da må vi vite hvordan ressurs registeret sorterer responsen utover UpdatedAt for å finne korrekt element i vår db for å lage token ut ifra. Denne problemstillingen er ikke løst med SubjectResourceLastUpdates tabellen heller, men det er kanskje ikke et stort problem. Jeg mener dette er en forenkling av koden, og at det plasserer ansvarsområdene på rett plass (infra huser sql queries, ikke app). @elsand Bare huk tak i meg om du har noen spørsmål eller vil sparre om løsningen 😁 Håper dette er til hjelp 🙂 PS - tok utgangspunkt i din gromme upsert metode når jeg lagde mergen! 🙌
MagnusSandgren
previously approved these changes
Sep 4, 2024
MagnusSandgren
previously approved these changes
Sep 4, 2024
oskogstad
reviewed
Sep 4, 2024
oskogstad
previously approved these changes
Sep 4, 2024
MagnusSandgren
dismissed stale reviews from oskogstad and themself
via
3841704
MagnusSandgren
previously approved these changes
Sep 4, 2024
Co-authored-by: Ole Jørgen Skogstad <skogstad@softis.net>
|
arealmaas
pushed a commit
that referenced
this pull request
Sep 10, 2024
🤖 I have created a release *beep* *boop* --- ## [1.17.0](v1.16.0...v1.17.0) (2024-09-10) ### Features * Add SubjectResource entity and db migration ([#1048](#1048)) ([d04d764](d04d764)) * **graphQL:** Add subscription for dialog details ([#1072](#1072)) ([8214acb](8214acb)) * Implement scalable dialog search authorization ([#875](#875)) ([aa8f84d](aa8f84d)) * revise dialog status ([#1099](#1099)) ([0029f46](0029f46)) ### Bug Fixes * ensure correct appsettings is used ([#1086](#1086)) ([d43f6d7](d43f6d7)) * ensure jobs are run with correct arguments and parameters ([#1085](#1085)) ([e21de56](e21de56)) * **webapi:** Return 422 when existing transmission IDs are used in dialog update ([#1094](#1094)) ([7a8a933](7a8a933)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
4 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This is the precursor to #875, adding the SubjectResource (was "RoleResource") entity and update mechanism that uses the new changefeed API in RR
This adds a "Janitor" console project, which can be invoked in container app jobs (or manually) to perform the syncronization.
Related Issue(s)
Verification
Documentation
docs-directory, Altinnpedia or a separate linked PR in altinn-studio-docs., if applicable)