Skip to content
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

Symfony messenger implementation #132

Merged
merged 8 commits into from Feb 10, 2020

Conversation

cv65kr
Copy link
Contributor

@cv65kr cv65kr commented Feb 6, 2020

This PR contains:

  • Bumped composer packages eg. Symfony 4.3 => 4.4
  • Removed some deprecations
  • Bumped docker-compose version
  • Used Symfony Messenger Bus instead league tactician-bundle

@cv65kr cv65kr force-pushed the feature/symfony-messenger-bus branch from e45f59c to 3ba8f07 Compare February 6, 2020 17:19
@cv65kr
Copy link
Contributor Author

cv65kr commented Feb 7, 2020

@jorge07 Did You know how resolve problem with tests?

@jorge07
Copy link
Owner

jorge07 commented Feb 7, 2020

Hi @cv65kr here the issue:

ERROR: Version in "./docker-compose.yml" is unsupported. You might be seeing this error because you're using the wrong Compose file version. Either specify a supported version (e.g "2.2" or "3.3") and place your service definitions under the `services` key, or omit the `version` key and place your service definitions at the root of the file to use version 1.

It's travis. I'm going to move to github actions asap.

BTW thanks for the contribution looks great

config/packages/twig.yaml Outdated Show resolved Hide resolved
@cv65kr
Copy link
Contributor Author

cv65kr commented Feb 7, 2020

@jorge07

I fixed docker issue yesterday, but there is a problem about tests which I describe in PR description.

If You look at App\Tests\Application\ApplicationTestCase we inject:

        $this->commandBus = $this->service('messenger.bus.command');
        $this->queryBus = $this->service('messenger.bus.query');

That's service are aliases for message buses, but this generate exceptions like below.

12) App\Tests\UI\Http\Rest\Controller\Events\GetEventsControllerTest::given_invalid_limit_returns_400_status
Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException: The "messenger.bus.command" service or alias has been removed or inlined when the container was compiled. You should either make it public, or stop using the container directly and use dependency injection instead.

Did You have any idea how resolve this problem?

@jorge07
Copy link
Owner

jorge07 commented Feb 7, 2020

hey @cv65kr just migrated to github actions as are way faster. Can you rebase please?

@jorge07
Copy link
Owner

jorge07 commented Feb 7, 2020

I think you need to define this services as public in the services_test.yaml

@cv65kr cv65kr force-pushed the feature/symfony-messenger-bus branch from 49ee33d to 79b5074 Compare February 7, 2020 21:52
@cv65kr
Copy link
Contributor Author

cv65kr commented Feb 7, 2020

Rebased.

I think you need to define this services as public in the services_test.yaml

I tried this but not working.

@jorge07
Copy link
Owner

jorge07 commented Feb 8, 2020

Awesome work here @cv65kr. Tests are failing due to depreciation but feel free to edit phpunit config to avoid this depreciations for now. We can address it in another PR along with the symfony 5 migration

@jorge07
Copy link
Owner

jorge07 commented Feb 8, 2020

One last question that I've regarding this symfony messenger switch is if we still need sncrabbitmq

@cv65kr
Copy link
Contributor Author

cv65kr commented Feb 8, 2020

One last question that I've regarding this symfony messenger switch is if we still need sncrabbitmq

For now yes, currently is related with Broadway stuff.

Green light from GA 👍

@jorge07
Copy link
Owner

jorge07 commented Feb 8, 2020

LGTM

Will do a full qa this night and if all goes well will finally merge.

Once again. @cv65kr thanks for the great work here.

Once merged I'll raise some issues to remove snc redis and have a bridge from Broadway to messenger, and another issue to start the v5 migration (may I rename the repository 😂)

@cv65kr cv65kr changed the title [WIP] Symfony messenger implementation Symfony messenger implementation Feb 8, 2020
@cv65kr cv65kr mentioned this pull request Feb 8, 2020
Copy link
Collaborator

@Lutacon Lutacon left a comment

Choose a reason for hiding this comment

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

LGTM

Just have a few questions and suggestions.

@jorge07 jorge07 merged commit 11cb261 into jorge07:master Feb 10, 2020
@jorge07
Copy link
Owner

jorge07 commented Feb 10, 2020

@cv65kr thanks you so much for the great work on this. I did a QA locally and all runs perfectly. @Lutacon thanks for your review!

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.

None yet

3 participants