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 algo stores to server from config #1156
Add algo stores to server from config #1156
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks very nice!
I put a few minor comments, and I have two highlights:
- I have just a doubt about the concept of having the server whitelisting themselves, because it sounds like bypassing the check of the store admin.
- I get now an error that I reported in the comments. I'm requesting the change just for this one (if it's not a mistake that I am making)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works perfectly!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added just a very last comment :)
algo_store_url = algo_store_url.replace( | ||
"localhost", "host.docker.internal" | ||
).replace("127.0.0.1", "host.docker.internal") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that this should then be changed like this
algo_store_url = algo_store_url.replace( | |
"localhost", "host.docker.internal" | |
).replace("127.0.0.1", "host.docker.internal") | |
algo_store_url = algo_store_url.replace( | |
"localhost", host_uri | |
).replace("127.0.0.1", host_uri) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ha thanks for spotting, that was a rather stupid mistake :-)
This PR makes sure that when config contains:
That the community store will be available for all collaborations in that server