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

[GH-33] implement auto detection mode for peering. #38

Merged
merged 7 commits into from
Apr 24, 2019

Conversation

s-soroosh
Copy link
Contributor

@s-soroosh s-soroosh commented Apr 22, 2019

Issue : #33 (only if appropriate)

@nolar I will write the tests and document it once we agreed on the implementation.

@zincr
Copy link

zincr bot commented Apr 22, 2019

🤖 zincr found 0 problems , 0 warnings

✅ Large Commits
✅ Approvals
✅ Specification
✅ Dependency Licensing

Copy link
Contributor

@nolar nolar left a comment

Choose a reason for hiding this comment

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

@psycho-ir Great work! Thanks. This should do the trick — I will try it out a bit later today.

There is only one important change request on the main() invocation. The rest are just the suggestions — for you to decide.

kopf/cli.py Outdated Show resolved Hide resolved
kopf/reactor/peering.py Show resolved Hide resolved
kopf/reactor/queueing.py Outdated Show resolved Hide resolved
kopf/reactor/peering.py Outdated Show resolved Hide resolved
@nolar nolar added the enhancement New feature or request label Apr 22, 2019
Copy link
Contributor

@nolar nolar left a comment

Choose a reason for hiding this comment

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

I've tried it locally. There is a little bug with the ...cls(peering=None,... — crashes on start (see comments). Once fixed, it works nice in all modes, exactly as intended.

Would you like to extend the docs (docs/peering.rsthttps://kopf.readthedocs.io/en/latest/peering/) in this PR? Or I can do this in the following PRs, together with other peering doc changes.

kopf/reactor/peering.py Outdated Show resolved Hide resolved
@nolar
Copy link
Contributor

nolar commented Apr 22, 2019

@psycho-ir PS: Also, feel free to remove the clutter from the PR body if you want. (I think, the PR template must be reduced to only an issue reference (it is mandatory), with no implicit structure. — Will do later.)

@s-soroosh
Copy link
Contributor Author

I've tried it locally. There is a little bug with the ...cls(peering=None,... — crashes on start (see comments). Once fixed, it works nice in all modes, exactly as intended.

Would you like to extend the docs (docs/peering.rsthttps://kopf.readthedocs.io/en/latest/peering/) in this PR? Or I can do this in the following PRs, together with other peering doc changes.

Sure, I will extend the docs tomorrow and try to add some tests for it.

nolar
nolar previously approved these changes Apr 22, 2019
@@ -11,9 +11,10 @@ The operator can be instructed to use alternative peering objects::

The operators from different peering objects do not see each other.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@nolar I updated the peering doc.

nolar
nolar previously approved these changes Apr 24, 2019
Copy link
Contributor

@nolar nolar left a comment

Choose a reason for hiding this comment

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

@psycho-ir Thanks.

if Peer._is_default_peering_setup():
return cls(peering=PEERING_DEFAULT_NAME, **kwargs)

logger.warning(f"The default peering object not found. Falling back to the Standalone mode...")
Copy link

@samurang87 samurang87 Apr 24, 2019

Choose a reason for hiding this comment

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

'* Default peering object not found, falling back to Standalone mode

kopf/reactor/peering.py Outdated Show resolved Hide resolved
@nolar nolar merged commit fa4d772 into zalando-incubator:master Apr 24, 2019
@nolar
Copy link
Contributor

nolar commented Apr 24, 2019

@psycho-ir So, it is merged and released as kopf==0.9. Congratulation! And big thanks for your contribution!

@s-soroosh
Copy link
Contributor Author

@nolar yaaaaaay. what is the next milestone? Would be happy to contribute more in this project.

@nolar
Copy link
Contributor

nolar commented Apr 26, 2019

@psycho-ir

Currently, the milestone 1 is this:

  • Tests, tests, tests — to bring the repo to a healthy state, so that I am not afraid to introduce new changes without breaking things. 90% of them are done, just in the PRs or in my local branches waiting for some PRs to be merged.

  • Silent spies on the events (see Silent handlers (spies) #30 ) — to react to the events in pods, persistent volume claims, etc, without storing the handler status. Already implemented in my local branch, waiting for the tests.

  • Finish the tutorial in the docs, so that the kind: EphemeralVolumeClaim becomes a real example operator in its own repo, uploaded to the DockerHub, etc. Partially drafted in the docs (in pieces), though not actually tested in real cluster. Waiting for the missing feature of the silent spies.


Since then, the framework is sufficiently feature-rich for the first stage (it is now, actually, just the docs do not feel complete), and can be advertised in public: meetups, blog posts, so on.

Based on the real-world feedback, the next milestones can be defined.

The real-world usage is the most important goal now. I.e., getting the operators implemented with this framework (and preferably shared).

Meanwhile, I write down all the ideas that come to my mind as the issues. If you have some suggestions, feel free to create the issues too. Examples: #44, #45, #46.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants