Skip to content
This repository has been archived by the owner on Sep 14, 2020. It is now read-only.

Use --standalone option in the readme of the examples #31

Closed
s-soroosh opened this issue Apr 21, 2019 · 5 comments
Closed

Use --standalone option in the readme of the examples #31

s-soroosh opened this issue Apr 21, 2019 · 5 comments
Labels
documentation Documentation improvements question Further information is requested

Comments

@s-soroosh
Copy link
Contributor

Hi,

First of all thanks a lot for initiating this project, I think it will be a very useful project for the community.

So about the issue, I had some difficulties to run the examples as I didn't setup the peering before jumping to the examples.

Firstly I think it would be better to use --standalone parameter in the readme of the examples, so that users who haven't setup the peering yet won't get error in first run.

Secondly I guess returning a more explicit error when the peering is not setup and referring to the relevant doc makes sense, I spent some time to debug the code and find the reason and fixed it. then I noticed it's documented here :D
https://kopf.readthedocs.io/en/latest/install/

wdyt? I woud be more than happy to contribute to it if you think these suggestions are sensible.

@nolar
Copy link
Contributor

nolar commented Apr 21, 2019

@psycho-ir Thanks for your feedback. I am glad that you like the idea of such a framework.

I think, your suggestion totally makes sense. If your suggestion is to fix the the docs & ./examples/*/README.md files only, feel free to do it — I welcome your contribution.

If you want to change the framework behaviour, please take a look at #32 & #33 first — I have just written down some of my thoughts on this topic.

The only thing that stops me from quickly implementing this, is the absence of tests. The framework has just recently left the proof-of-concept stage, and I cover it with the tests component-by-component, module-by-module (not fast enough, sorry).

@nolar nolar added documentation Documentation improvements question Further information is requested labels Apr 21, 2019
@s-soroosh
Copy link
Contributor Author

I just read them, yep they make totally sense to me.
I think by implementing #33 we don't even to change the doc and adding the --standalone to the docs.

I can work on it this week, or you would rather it's implemented after #32? to have support of the namespaced peerings in the first run.

@nolar
Copy link
Contributor

nolar commented Apr 21, 2019

@psycho-ir I would say they are independent (unless there are some tricky details which I do not foresee).

You can just keep the always-cluster-scoped concept for now, and implement the logic for the present/absent peering object.

This was referenced Apr 22, 2019
@nolar
Copy link
Contributor

nolar commented Apr 22, 2019

@psycho-ir Please, take a look at #36 & #37 — regarding the #32 (namespace isolation of the served objects and of the peering objects). Seems working as expected — I will double-check tomorrow, with fresh mind.

You can send your implementation for #33 as is — I will rebase&resolve these two PRs later (they are less important than a working tutorial).

@nolar
Copy link
Contributor

nolar commented Apr 26, 2019

Indirectly fixed by #33 & #38 (auto-detection of the peering mode) — the tutorial does not break if the cluster is not configured with the peering object.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
documentation Documentation improvements question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants