Skip to content

Conversation

@docjyJ
Copy link

@docjyJ docjyJ commented Sep 30, 2025

Summary

This allows you to manually define an LDAP configuration prefix. This is useful for scripts.

The scripts know the prefix and can manage their configuration without interacting with other configurations.

Checklist

@docjyJ docjyJ requested a review from a team as a code owner September 30, 2025 09:41
@docjyJ docjyJ requested review from nfebe, provokateurin and salmart-dev and removed request for a team September 30, 2025 09:42
@docjyJ
Copy link
Author

docjyJ commented Sep 30, 2025

This is my first PR on the repo server, so please feel free to give feedback.

@szaimen szaimen added enhancement 3. to review Waiting for reviews labels Sep 30, 2025
@szaimen szaimen added this to the Nextcloud 33 milestone Sep 30, 2025
@szaimen szaimen requested review from blizzz and come-nc September 30, 2025 09:49
Copy link
Contributor

@come-nc come-nc left a comment

Choose a reason for hiding this comment

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

Why not use the returned prefix in your script instead?
When using occ ldap:create-empty-config --only-print-prefix, the only output is the prefix and you can then store that in a variable in your script.

@docjyJ
Copy link
Author

docjyJ commented Sep 30, 2025

Why not use the returned prefix in your script instead?
When using occ ldap:create-empty-config --only-print-prefix, the only output is the prefix and you can then store that in a variable in your script.

There are two reasons related to the operation of AIO community containers:

  • You can't use VAR=$(cmd), is that right, @szaimen?
  • The configuration updates each time the system is restarted.

For these reasons, it would be useful to have an identifier known in advance.

Add option to manually set configuration prefix with validation.

Signed-off-by: Jean-Yves <[email protected]>
Signed-off-by: Jean-Yves <[email protected]>
@docjyJ
Copy link
Author

docjyJ commented Sep 30, 2025

Adding this feature could lead to inappropriate behaviour if a user creates an s99 configuration and then a configuration with an automatically generated prefix.

Maybe prefixes should be generated in a way that makes them unique (e.g. uuid).

@szaimen
Copy link
Contributor

szaimen commented Oct 1, 2025

  • You can't use VAR=$(cmd), is that right, @szaimen?

Currently you cannot afaik but I guess we could add support for it...

@docjyJ
Copy link
Author

docjyJ commented Oct 1, 2025

Currently you cannot afaik but I guess we could add support for it...

That shouldn't solve the update problem.

@github-actions
Copy link
Contributor

Hello there,
Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.

We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process.

Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6

Thank you for contributing to Nextcloud and we hope to hear from you soon!

(If you believe you should not receive this message, you can add yourself to the blocklist.)

@come-nc
Copy link
Contributor

come-nc commented Oct 20, 2025

Adding this feature could lead to inappropriate behaviour if a user creates an s99 configuration and then a configuration with an automatically generated prefix.

Maybe prefixes should be generated in a way that makes them unique (e.g. uuid).

For this reason and other potential pitfalls I prefer to stay away from manually set prefix, LDAP configuration prefixes should stay automated.
The command already outputs the new prefix to help with scripting.

@come-nc come-nc closed this Oct 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants