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

fix: default config #179

Merged
merged 2 commits into from
Sep 2, 2021
Merged

fix: default config #179

merged 2 commits into from
Sep 2, 2021

Conversation

AbelVandenBriel
Copy link
Contributor

No description provided.

@AbelVandenBriel AbelVandenBriel self-assigned this Sep 1, 2021
@AbelVandenBriel AbelVandenBriel added the bug Something isn't working label Sep 1, 2021
"@id": "urn:semcom-node:default:MemoryStore",
"@type": "MemoryStore",
"initialData": ["peers", []],
"initialData": ["storage", ["http://localhost:9000/metadata/"]]
Copy link
Contributor

Choose a reason for hiding this comment

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

If I understand correctly; this config is used to start the repository in production. If so; wouldn't this break something? Because I presume there's no pod server available in production on this uri.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hm, the point is there should be a pod server deployed with it.

Copy link
Contributor

Choose a reason for hiding this comment

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

(or in any case, there should somewhere be a pod server to which this points, where the metadata is stored)

Copy link
Contributor

Choose a reason for hiding this comment

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

Could we use an in-memory store as fallback?

Copy link
Contributor

Choose a reason for hiding this comment

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

There was an inmemory config, but it does not yet work with the sync part.

Copy link
Contributor

Choose a reason for hiding this comment

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

Might be a justifiable trade-off in this specific case?

Copy link
Contributor

Choose a reason for hiding this comment

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

The problem is that, since it does not include the sync handlers, the launch code fails when those are not found.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we could solve that with a boolean config variable?

Copy link
Contributor

@woutermont woutermont Sep 1, 2021

Choose a reason for hiding this comment

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

Wouldn't it be better just to deploy a pod server with it? That is also the only way it's really useable (with the ability to upload new components with oidc auth etc.)

@woutermont woutermont merged commit 5a4e1b9 into develop Sep 2, 2021
@woutermont woutermont deleted the fix/default-config branch September 2, 2021 16:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants