Skip to content

Conversation

@John-Memphis
Copy link
Contributor

Search for "TO:DO" (no quotes) in the file. There's a part that needs to be double checked for clarity about what the words actually mean.

@shay23b shay23b requested a review from yanivbh1 October 21, 2023 15:33
@John-Memphis John-Memphis changed the title chore: added the extra examples from academy to the python readme and cleaned up some of the working on things chore: added the extra examples from academy to the python readme Oct 22, 2023
@idanasulin2706
Copy link
Contributor

@yanivbh1 please review

README.md Outdated

For details on deploying memphis open-source with different types of connections see the [docs](https://docs.memphis.dev/memphis/memphis-broker/concepts/security).

The default connection type (for cloud) is using a JWT token-based connection. Here is an example of using memphis.connect with a JWT token:
Copy link
Contributor

Choose a reason for hiding this comment

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

@idanasulinmemphis @shay23b not sure I understand this comment. Also, the default connection method is password-based.

Copy link
Contributor

Choose a reason for hiding this comment

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

@John-Memphis I dont see any example with account id for connecting with the Cloud

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@yanivbh1 I'll add an example for that.
image
image

All of the repos say that the default connection is token based. I've only been using Memphis with open-source and deployed with docker--where password is the default. I was assuming it was a kubernetes thing or a memphis cloud thing that used tokens by default.

If that's not the case I'll go and change that in all of the repos.

partitions_number=1, # defaults to 1
dls_station="<station-name>" # defaults to "" (no DLS station) - If selected DLS events will be sent to selected station as well
)
**A station will be automatically created for the user when a consumer or producer is used if no stations with the given station name exist.**<br><br>
Copy link
Contributor

Choose a reason for hiding this comment

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

@John-Memphis , please add a phrase on what a station is

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

Copy link
Contributor

@yanivbh1 yanivbh1 left a comment

Choose a reason for hiding this comment

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

Comments inside

@John-Memphis
Copy link
Contributor Author

Synced with master, addressed comments from @yanivbh1 and changed some of the sentences throughout for a better reading experience.

This readme update should be ready.

@idanasulin2706
Copy link
Contributor

@John-Memphis for me that looks ok, @yanivbh1 please take a look as well

@yanivbh1
Copy link
Contributor

@idanasulinmemphis looks great to me

@idanasulin2706 idanasulin2706 merged commit 9c76e07 into master Oct 24, 2023
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.

4 participants