Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Add a documentation page for lock in FW#12523

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
wouterj merged 1 commit intosymfony:4.4fromjderusse:lock-fw
Oct 4, 2020

Conversation

@jderusse
Copy link
Member

@jderussejderusse commentedOct 21, 2019•
edited
Loading

This PR add a dedicated page to explain how to use Lock with FrameworkBundle

This PR slightly depends onsymfony/symfony#34043 (PDO and Zookeeper DSN)

I should have pushed this PR years ago 😢 sorry about that...

Fixes:#12561

@jderusse
Copy link
MemberAuthor

RFR

Copy link
Contributor

@clemherremanclemherreman left a comment

Choose a reason for hiding this comment

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

If I were a real reviewer, I'd say this looks good to merge :) Thank you@jderusse

OskarStark reacted with thumbs up emoji
@jderussejderusseforce-pushed thelock-fw branch 5 times, most recently from825534d tod8a8416CompareApril 18, 2020 10:47
Copy link
Contributor

@maxheliasmaxhelias left a comment

Choose a reason for hiding this comment

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

Thanks for the reminder, i forgot some notion 😁

Copy link
Member

@wouterjwouterj left a comment

Choose a reason for hiding this comment

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

Sorry for completely missing this PR@jderusse! I think it's a very great step (we're trying to move all important information from the component to the main guides in the docs).

I've left a few comments, most of them are minor and can be fixed while merging. So don't worry if you don't have time to fix them now :) Imho, it's ready for merge

lock.rst Outdated

The same instance of ``LockInterface`` won't block when calling `acquire``
multiple times. If several services share the same ``lock`` service, they
won't lock each other, use ``LockFactory`` instead.
Copy link
Member

Choose a reason for hiding this comment

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

I don't really understand this caution. The above example, when run concurrentlywill block eachother, right?

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

Depends, what you callconcurrently, when running currently in the same process (think react-php), the same instance of Lock (think shared service) wont block.

Bellow case where lock does not apply

$lock =newLock();$lock->acquire();$lock->acquire();// no lock (same instance)
$lock =newLock();$a =newReactControler($lock);$b =newReactControler($lock);// Both $a and  share same lock instance and wont block each other

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

I would appreciate help to rephrase my sentence :)

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

reworded

Copy link
Member

@wouterjwouterjOct 4, 2020•
edited
Loading

Choose a reason for hiding this comment

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

Thanks for clarifying! I've revisited this caution in1170a7c . Can you please check if the new contents of this caution are correct?

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

sounds good.
Thanks for your help

@wouterjwouterj changed the base branch frommaster to4.4October 4, 2020 11:00
@wouterj
Copy link
Member

Thanks a lot Jérémy! This PR has been forgotten about for to many times, my apologies. It's now merged into the 4.4 version of the docs.

@wouterjwouterj merged commit2900e50 intosymfony:4.4Oct 4, 2020
wouterj added a commit that referenced this pull requestOct 4, 2020
wouterj added a commit that referenced this pull requestOct 4, 2020
* 4.4:  [#11009] Some tweaks  [DependencyInjection] Doc for #30257 Allow to choose an index for tagged collection  [#12523] Reworded caution  Add a documentation page for lock in FW  [Configuration] Add documentation about `ignore_errors: not_found` option.
wouterj added a commit that referenced this pull requestOct 4, 2020
* 5.1:  [#11009] Some tweaks  [DependencyInjection] Doc for #30257 Allow to choose an index for tagged collection  [#12523] Reworded caution  Add a documentation page for lock in FW  [Configuration] Add documentation about `ignore_errors: not_found` option.
@xabbuhxabbuh added this to the4.4 milestoneOct 5, 2020
@xabbuhxabbuh added the Lock labelOct 5, 2020
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@wouterjwouterjwouterj approved these changes

@OskarStarkOskarStarkOskarStark requested changes

@HeahDudeHeahDudeAwaiting requested review from HeahDude

@xabbuhxabbuhAwaiting requested review from xabbuh

+2 more reviewers

@clemherremanclemherremanclemherreman left review comments

@maxheliasmaxheliasmaxhelias approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Projects

None yet

Milestone

4.4

Development

Successfully merging this pull request may close these issues.

[Lock] Add missing lock connection string in FrameworkExtension

7 participants

@jderusse@wouterj@clemherreman@OskarStark@maxhelias@xabbuh@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp