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

Feat/seed accounts and pods #1165

Merged
merged 7 commits into from
Mar 10, 2022

Conversation

adlerfaulkner
Copy link
Member

@adlerfaulkner adlerfaulkner commented Feb 16, 2022

πŸ“ Related issues

#1129

✍️ Description

Adds the ability to seed non root pods and associated accounts. Adds the --seededPodConfigJson cli arg when starting the server which defaults to ./seeded-pod-config.json.

Adds SeededPodInitializer.ts which mimics a lot of the functionality of RegistrationManager.ts (Maybe this should be DRYed up?)

Can be run using the file-seeded.json config file.

Wanted to be able to get my dev env setup faster.

βœ… PR check list

Before this pull request can be merged, a core maintainer will check whether

  • this PR is labeled with the correct semver label
    • semver.patch: Backwards compatible bug fixes.
    • semver.minor: Backwards compatible feature additions.
    • semver.major: Breaking changes. This includes changing interfaces or configuration behaviour.
  • the correct branch is targeted. Patch updates can target main, other changes should target the latest versions/* branch.
  • the RELEASE_NOTES.md document in case of relevant feature or config changes.

@adlerfaulkner adlerfaulkner changed the title feat(seeding): seed accounts and pods with seeded-pod-config.json Feat: seed accounts and pods Feb 16, 2022
@adlerfaulkner adlerfaulkner changed the title Feat: seed accounts and pods Feat/seed accounts and pods Feb 16, 2022
@joachimvh
Copy link
Member

I first want to talk about what your use case is because I think you're making this more difficult for yourself than this should be. My guess is that you want the server to immediately set up a number of pods, based on a simple list containing just the pod name and the login information right?

As mentioned in #1129, the ConfigPodInitializer is only relevant when you have a very custom setup where every pod uses a different Components.js configuration. This includes the dependendent classes such as TemplatedResourcesGenerator which is used here.

In case you want to add pods to an existing server, the following script would do that for you:

const users = [
  { name: 'example', email: 'hello@example.com', password: 'abc123' }
]
const results = [];
for (const user of users) {
  const json = {
    podName: user.name,
    email: user.email,
    password: user.password,
    confirmPassword: user.password,
    createWebId: true,
    register: true,
    createPod: true,
  };
  const response = await fetch('http://localhost:3000/idp/register/, {
    method: 'POST'
    headers: { 'content-type': 'application/json' },
    body: JSON.stringify(json);
  }
  results.push(await response.json());
}

After that the results array would contain all the necessary information such as pod URLs and WebIDs. (I typed all this in a text editor so I expect there to be a syntactic error in there).

This is still an external script though. In case this is added to the server itself (and I could see the need), it would indeed have to be some kind of Initializer that triggers only once. But its implementation would be quite concise: a loop that registers the users similarly to how the SetupHandler does this https://github.com/solid/community-server/blob/ce754c119fb87dc8a4f79c639e316bd04d40109b/src/init/setup/SetupHandler.ts#L76-L81

Unless I'm missing something else here that needs to be done.

Besides all that, the versions/3.0.0 branch drastically changes how CLI parameters are used so that should be the target branch.

@adlerfaulkner
Copy link
Member Author

adlerfaulkner commented Feb 16, 2022

@joachimvh thanks for the quick response!

My guess is that you want the server to immediately set up a number of pods, based on a simple list containing just the pod name and the login information right?

Yes, this is what I wanted to accomplish. I am coming from a Ruby on Rails background where developers can seed data through a single file (seeds.rb) and command (rake db:seed). When trying to quickly prototype, it's a pain/time sink to have to manually enter information into the setup manager interface and the registration interface every time you want to "reset" to a clean slate.

From #1129 I understood that I could have created an external script like the one you posted to seed data. However, as a developer trying to use this codebase for the first time, I didn't know what the registration endpoint was nor what it's arguments were. So, in trying to understand how the code worked more fully (and as prep for potentially developing with CSS a lot more in the future), I figured I would just modify/extend it to do what I wanted and learn the framework.

I hadn't seen/found SetupHttpHandler.ts yet, but I can see how I could have simplified SeededPodInitializer.ts to just use the registrationManager instead of copying its functionality.

I think on a more general note it would be beneficial to have more documentation on how developers can get started with CSS. For example, from my 1 day of using the codebase:

  • It's great that there's a built in setup manager for the developer to initialize their server via an interface, but what about the next step when the developer wants to remove the setup manager interface and initialize their server via config alone. To do so, they have to get up to speed on Components.js, then figure out the world of possibilities in the config folder. Modifying the config file requires a pretty thorough understanding of the codebase. How can we decrease the jump or create a more stepwise transition to get a developer from (1) using the setup manager interface to (2) modifying the config file to run their own initializers, templates, generators, etc.
  • Some documentation about the use of handlebars in templating resources. (Did I miss this?)
  • Some way to view all the configured routes (again, coming from RoR where one can run rake routes to get this info) (maybe I'm missing something where this knowledge is embeded in the solid specs?)
  • I wasn't aware that the versions/3.0.0 branch existed nor the drastic changes to cli params it's bringing about are, nor what it's stability/progress is. Is there someway I could have found that information out? Maybe some mention of that in the README would be beneficial?

@joachimvh
Copy link
Member

I didn't know what the registration endpoint was nor what it's arguments were

When you start the server for the first time, there should be a link to the registration page on the main page.

I think on a more general note it would be beneficial to have more documentation on how developers can get started with CSS. For example, from my 1 day of using the codebase:

But yes, this is the main point and I fully agree with it. It's the usual issue of needing time for both documentation and features, and features usually winning that competition. There are some more people planning to work on the server in the future so hopefully that will allow us to solve this problem a bit better.

  • It's great that there's a built in setup manager for the developer to initialize their server via an interface, but what about the next step when the developer wants to remove the setup manager interface and initialize their server via config alone. To do so, they have to get up to speed on Components.js, then figure out the world of possibilities in the config folder. Modifying the config file requires a pretty thorough understanding of the codebase. How can we decrease the jump or create a more stepwise transition to get a developer from (1) using the setup manager interface to (2) modifying the config file to run their own initializers, templates, generators, etc.

The main part of the server that does have (a small bit of) documentation is the configuration part. The /config folder contains a README with some info on how to customize the config, and all the subfolders have their own README giving some information about what the possible options do. Specifically to disable the setup you can use one of the no-setup configs that are in the /config folder, or for example compare them to the other version to see how they differ.

  • Some documentation about the use of handlebars in templating resources. (Did I miss this?)

Not sure what you need here. We happen to use handlebars for the files that are generated in the pods to write some simple fields. Unless you mean a specific list of the fields that are exported?

  • Some way to view all the configured routes (again, coming from RoR where one can run rake routes to get this info) (maybe I'm missing something where this knowledge is embeded in the solid specs?)

With routes, do you mean the URLs supported by the server? Because due to Solid that is an infinite amount. Although we should have a documented list for some of the IDP APIs.

  • I wasn't aware that the versions/3.0.0 branch existed nor the drastic changes to cli params it's bringing about are, nor what it's stability/progress is. Is there someway I could have found that information out? Maybe some mention of that in the README would be beneficial?

When opening a PR the pre-generated comments mention targeting the correct branch. We should probably specify what the options there are (and again, have more documentation about this).

So yes, most of it boils down to the fact that more documentation would definitely be helpful. Comments like this also help because they indicate what kind of documentation would be needed so thanks for that.

@adlerfaulkner
Copy link
Member Author

When you start the server for the first time, there should be a link to the registration page on the main page.

Yep, though this is a pretty tedious way to learn how every endpoint works and what arguments it accepts.

It's the usual issue of needing time for both documentation and features, and features usually winning that competition.

Of course, no way to get around time.

The main part of the server that does have (a small bit of) documentation is the configuration part.

Ah yes, I saw and reviewed the README in the top level config folder but may not have seen all the ones in the sub folders.

Some documentation about the use of handlebars in templating resources. (Did I miss this?)

Not sure what you need here. We happen to use handlebars for the files that are generated in the pods to write some simple fields.

I guess it's just that I had to inspect templates/pod/profile/card$.ttl.hbs to see what fields were required (after I figured out how templates were being used), then figure out how those fields were being supplied through the settings arg of createPod within a PodManager.

With routes, do you mean the URLs supported by the server? Because due to Solid that is an infinite amount. Although we should have a documented list for some of the IDP APIs.

Yeah I mean URLs supported by the server. And yeah, IdP side specifically. I understand how that's not possible for all the URLs available in pods.

When opening a PR the pre-generated comments mention targeting the correct branch.

Right. I saw that once I opened the PR but by then I had already done all my development based on main.

@adlerfaulkner adlerfaulkner changed the base branch from main to versions/3.0.0 February 18, 2022 23:18
@adlerfaulkner
Copy link
Member Author

adlerfaulkner commented Feb 18, 2022

@joachimvh If you/the maintainers are interested in incorporating this feature:

  • I rebased with version/3.0.0, applied my changes to the new way that cli args are resolved, and changed the base branch
  • I simplified SeededPodInitializer to just use RegistrationManager the same way that SetupHandler does

If you don't want to incorporate this feature, what's the process of turning it into an extension? Is there documentation somewhere for that?

Also: I'd love to help out where I can with documentation, starting with this feature, if it doesn't already have enough. Do you have a framework/guidelines in place for or references to documentation best practices?

@joachimvh
Copy link
Member

If you don't want to incorporate this feature, what's the process of turning it into an extension? Is there documentation somewhere for that?

The discussion here is relevant #1163

Also: I'd love to help out where I can with documentation, starting with this feature, if it doesn't already have enough. Do you have a framework/guidelines in place for or references to documentation best practices?

I opened issue #1174 so we have something to so we have a place to track this, but as you can see there is not much there yet.

Regarding this PR, I do think this could be a useful addition to this repository as other people have also asked how to create a set of pods. I will have a look at the implementation a bit later.

@joachimvh joachimvh self-requested a review February 21, 2022 11:00
Copy link
Member

@joachimvh joachimvh left a comment

Choose a reason for hiding this comment

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

Most comments are config related. I think this can be made even more flexible. Let me know if you need help with the configuration as I know it can be a hassle sometimes.

config/README.md Outdated Show resolved Hide resolved
config/README.md Outdated Show resolved Hide resolved
config/app/init/initialize-seeded-root.json Outdated Show resolved Hide resolved
config/app/init/initializers/seeded-root.json Outdated Show resolved Hide resolved
config/app/variables/cli/cli.json Outdated Show resolved Hide resolved
config/app/variables/resolver/resolver.json Outdated Show resolved Hide resolved
config/util/variables/default.json Outdated Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
config/app/init/initializers/seeded-root.json Outdated Show resolved Hide resolved
src/init/SeededPodInitializer.ts Outdated Show resolved Hide resolved
@adlerfaulkner
Copy link
Member Author

adlerfaulkner commented Mar 7, 2022

@joachimvh
Sorry it took me a while to get back to this.

To make the tests pass after implementing your suggestions I had to make a few modifications:

  • Integration tests:
    Had to add some imports to the configs in test/integration/config to make sure that every app instantiated by the tests had defined the dependencies of SeededPodInitializer, namely IdentifierGenerator, OwnershipValidator, AccountStore and PodManager.

  • AssetPathResolver:
    When I just removed defaultPath as you suggested here, I get:

    2022-03-07T18:22:48.451Z [Components.js] info: Initiating component discovery from /Users/adlerfaulkner/community-server/
    2022-03-07T18:22:48.796Z [Components.js] info: Discovered 136 component packages within 1282 packages
    2022-03-07T18:22:48.798Z [Components.js] info: Initiating component loading
    2022-03-07T18:22:51.240Z [Components.js] info: Registered 493 components
    2022-03-07T18:22:51.241Z [Components.js] info: Loaded configs
    2022-03-07T18:22:53.012Z [Components.js] error: Detected fatal error. Generated 'componentsjs-error-state.json' with more information.
    Could not create the server
    Cause: Undefined variable: urn:solid-server:default:variable:seededPodConfigJson
    Error: Undefined variable: urn:solid-server:default:variable:seededPodConfigJson
    

    Because componentsjs throws an error at undefined variables (see componentsjs docs).

    If a variable remains undefined, an exception will be thrown during instantiation.

    If we make defaultPath an empty string AssetPathResolver tries to resolve it into a file path and we get:

    2022-03-07T20:02:19.134Z [Components.js] info: Initiating component discovery from /Users/adlerfaulkner/community-server/
    2022-03-07T20:02:19.489Z [Components.js] info: Discovered 136 component packages within 1282 packages
    2022-03-07T20:02:19.491Z [Components.js] info: Initiating component loading
    2022-03-07T20:02:22.119Z [Components.js] info: Registered 493 components
    2022-03-07T20:02:22.119Z [Components.js] info: Loaded configs
    2022-03-07T20:02:24.264Z [AppRunner] error: Could not start the server: EISDIR: illegal operation on a directory, read
    Could not start the server
    Cause: EISDIR: illegal operation on a directory, read
    Error: EISDIR: illegal operation on a directory, read
    

    I had to make it return null in the case that neither a value nor a defaultPath are set.

@adlerfaulkner
Copy link
Member Author

Looks like I got some commits from main in here that are not in versions/4.0.0. They should be anyways right?

@joachimvh
Copy link
Member

Looks like I got some commits from main in here that are not in versions/4.0.0. They should be anyways right?

They have to get in there eventually, but doing it through an unrelated PR is going to potentially cause some git issues/confusion so please remove them and do a force push.

Copy link
Member

@joachimvh joachimvh left a comment

Choose a reason for hiding this comment

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

Looks great and pretty much ready to merge besides some minor nits (and the commits from the main branch). I see one integration test is still failing. That is because that one only runs on CI, so the run-with-redlock.json config will also need the config changes that you added to the other ones.

guides/seeding-pods.md Show resolved Hide resolved
guides/seeding-pods.md Outdated Show resolved Hide resolved
src/init/SeededPodInitializer.ts Outdated Show resolved Hide resolved
test/unit/init/SeededPodInitializer.test.ts Outdated Show resolved Hide resolved
@joachimvh
Copy link
Member

I have merged the main branch into the 4.0.0 branch in the hopes that makes it easier, I hope that didn't make it more difficult for you.

@adlerfaulkner
Copy link
Member Author

@joachimvh not sure what's up with the CI tests now. I added the changes to run-with-redlock.json

@joachimvh
Copy link
Member

@joachimvh not sure what's up with the CI tests now. I added the changes to run-with-redlock.json

CI was acting up, tests succeed after running them again.

Copy link
Member

@joachimvh joachimvh left a comment

Choose a reason for hiding this comment

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

Looks good, thanks for this!

Just realized a note should be added to the release notes about this but I'll do it myself in a separate commit.

@joachimvh joachimvh merged commit c8d4bfe into CommunitySolidServer:versions/4.0.0 Mar 10, 2022
@adlerfaulkner
Copy link
Member Author

πŸŽ‰ Thanks for all the help & review @joachimvh ! Excited to be contributing.

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.

None yet

2 participants