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

Add createRealmLineage mutation to API #1179

Merged
merged 1 commit into from
Sep 12, 2024

Conversation

LukasKalbertodt
Copy link
Member

This is supposed to make mounting series easier from external scripts. The existing mountSeries API is (a) doing a lot of different things and (b) requires the caller to distinguish between what realms already exist and which still need to be created. This new API is idempotent and more convenient. With this, it would be possible to simplify mountSeries, but we can't simply break that API since the Admin UI is using it. So we keep it for now. We might add an alternative API in the future.

@wsmirnow asked for this. Please let me know if this already helps you! You still have to use the mountSeries API but at least you can always pass [] as new_realms by first calling this new API.

@LukasKalbertodt LukasKalbertodt added the changelog:admin Changes primarily for admins label Jun 12, 2024
@github-actions github-actions bot temporarily deployed to test-deployment-pr1179 June 12, 2024 11:57 Destroyed
@github-actions github-actions bot added the status:conflicts This PR has conflicts that need to be resolved label Jun 20, 2024

This comment was marked as resolved.

@github-actions github-actions bot removed the status:conflicts This PR has conflicts that need to be resolved label Jun 25, 2024
@github-actions github-actions bot temporarily deployed to test-deployment-pr1179 June 25, 2024 08:24 Destroyed

This comment was marked as resolved.

@github-actions github-actions bot added the status:conflicts This PR has conflicts that need to be resolved label Jul 1, 2024
This is supposed to make mounting series easier from external scripts.
The existing `mountSeries` API is (a) doing a lot of different things
and (b) requires the caller to distinguish between what realms already
exist and which still need to be created. This new API is idempotent and
more convenient. With this, it would be possible to simplify
`mountSeries`, but we can't simply break that API since the Admin UI
is using it. So we keep it for now. We might add an alternative API in
the future.
@github-actions github-actions bot removed the status:conflicts This PR has conflicts that need to be resolved label Jul 1, 2024
@github-actions github-actions bot temporarily deployed to test-deployment-pr1179 July 1, 2024 12:42 Destroyed
@wsmirnow
Copy link
Contributor

Hi @LukasKalbertodt, this PR goes in the right direction. But… this is not really mkdir -p behavior. Here an example:

There are following realms in Tobira:

  • Lectures
    • 2024
      • Spring
        • Lecture 1
        • Lecture 2

In my script I want to add a new Leaf and mount a Series in a specific path like that:

  • Lectures
    • 2024
      • Spring
        • Lecture 1
        • Lecture 2
      • Autumn
        • Lecture 3

The base path (or parent realm path) is configured to be /lectures and the sub path (new realms path) would look like:

[
      {name: "2024", pathSegment: "2024"},
      {name: "Autumn", pathSegment: "autumn"}, 
      {name: "Lecture 3", pathSegment: "lecture3"}
]

I would expect, that all paths on the way would be created with pathSegment and name if they aren't exists. But if they exists, at least with the same pathSegment, create the next leaf until the end of list.

But the current handling is to throw an exception with message: Invalid input: realm with that path already exists. In this case I have to check the whole path to know, what is my parentRealmPath, on every request.

@LukasKalbertodt
Copy link
Member Author

The base path (or parent realm path) is configured to be /lectures

I don't understand this part. The new API (createRealmLineage) does not have any notion of "base path" or "parent realm path". It just have one argument, a list of realms. In your example, you would pass:

[
      {name: "Lectures", pathSegment: "lectures"},
      {name: "2024", pathSegment: "2024"},
      {name: "Autumn", pathSegment: "autumn"}, 
      {name: "Lecture 3", pathSegment: "lecture3"}
]

I.e. always pass the whole path. And then it should have exactly the behavior you want: only autumn and lecture3 are created and no error is thrown.

After having called createRealmLineage like that, you can use the existing mountSeries API: pass the target realm as parentRealmPath argument (yeah, the name is not great then) and pass an empty array for newRealms.

@wsmirnow
Copy link
Contributor

wsmirnow commented Sep 11, 2024

Apparently I tested mountSeries mutation, shame on me. I've now looked at createRealmLineage mutation. It works as it should.

@owi92 owi92 merged commit aa6f0e9 into elan-ev:master Sep 12, 2024
4 checks passed
@LukasKalbertodt LukasKalbertodt deleted the better-mount-api branch September 12, 2024 11:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog:admin Changes primarily for admins
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants