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

Consume mandatees from LMB #190

Merged
merged 17 commits into from
Sep 9, 2024
Merged

Consume mandatees from LMB #190

merged 17 commits into from
Sep 9, 2024

Conversation

lagartoverde
Copy link
Contributor

@lagartoverde lagartoverde commented Aug 15, 2024

Overview

This implements the LDES consumer which takes mandatees created on LMB and adds them to GN, In the corresponding public and private graphs

connected issues and PRs:

GN-4980

Setup

You will need to replace the extra headers env variable, ask me for the auth token.
Also is probably your dev setup of virtuoso doesn't have enough ram, if you are running into out of memory problems increase it

How to test/reproduce

Consume the data, check that now you have mandatees in http://mu.semte.ch/graphs/lmb-data-public, you should be able to find these mandatees in the frontend if you go to the corresponding administrative units

Challenges/uncertainties

Waiting for a Niels answer related to in which graphs should be put de data

Checks PR readiness

  • UI: works on smaller screen sizes
  • UI: feedback for any loading/error states
  • Check cancel/go-back flows
  • Check database state correct when deleting/updating (especially regarding relationships)
  • changelog
  • no new deprecations

Copy link
Contributor

@elpoelma elpoelma left a comment

Choose a reason for hiding this comment

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

The mandatees and their related data seem to be ingested well :) Really nice work. They also correctly show up on the frontend.
Some notes:

  • I had to use a different virtuoso.ini files (with different query and RAM limits), which I got from @lagartoverde
  • We should probably clear the cache after the postprocessing step

I have not yet been able to test what happens when adding a new mandatee to the LMB application.

@elpoelma
Copy link
Contributor

Additionally, you should probably also add the following entry {"variables":[],"name":"lmb-public"} to the DEFAULT_MU_AUTH_ALLOWED_GROUPS_HEADER env var.

@elpoelma
Copy link
Contributor

Tested, with the additional DEFAULT_MU_AUTH_ALLOWED_GROUPS_HEADER:

  • It seems that adding a new mandatee to the lmb application seems to more-or-less go through (there still seem to be some issues on the side of lmb). As expected, I do have to restart the cache
  • Unfortunately, it also seems the database contains a lot of duplicate triples:
    image

@elpoelma elpoelma self-requested a review August 29, 2024 09:25
Copy link
Member

@abeforgit abeforgit left a comment

Choose a reason for hiding this comment

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

this now works on my machine
It threw an error earlier today, but I tried again with a fresh qa backup and it ran without issue, so 🤷

@elpoelma elpoelma self-requested a review September 3, 2024 14:12
Copy link
Contributor

@elpoelma elpoelma left a comment

Choose a reason for hiding this comment

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

This now works well on my end (with the beefy virtuoso.ini of course).
For this feature to work correctly a beefier virtuoso.ini is needed, as the service needs to be able to make queries/updates above 10000 triples.

I can correctly see the new mandatees appearing in the frontend.

I did notice there are still two ununused queries:

  • moveIdentifiersToPersonStagingQuery
  • moveBirthdaysToPersonStagingQuery

I would still omit the identifier property (even from the private graph), as we do not seem to require it.

@lagartoverde lagartoverde merged commit d0d7e38 into master Sep 9, 2024
@lagartoverde lagartoverde deleted the feat/js-ldes-client branch September 9, 2024 07:23
@lagartoverde lagartoverde restored the feat/js-ldes-client branch September 9, 2024 08:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants