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.

Sign up for GitHub

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

Jump to bottom

427: configuration updates message broker #4

Merged
merged 14 commits into from
Dec 16, 2020
Merged

427: configuration updates message broker #4

merged 14 commits into from
Dec 16, 2020

Conversation

owlsmage
Copy link
Contributor

@owlsmage owlsmage commented Nov 29, 2020 •
edited
Loading

Сonfiguration import message broker

Description (*)

Module with queue consumer that provides possibility for services to subscribe for configuration changes and receive changed configuration

Related Pull Requests

magento/commerce-data-export#15
https://github.com/magento-commerce/magento2-infrastructure/pull/1395

Fixed Issues (if relevant)

  1. Fixes Configuration propagation catalog-storefront#427

Code Review Checklist (*)

See detailed checklist

  • Story AC is completed
  • proposed changes correspond to Magento Technical Vision
  • new or changed code is covered with web-api/integration tests (if applicable)
    • expected results in test verified with data from fixture
  • no backward incompatible changes
  • Class usage: magento/storefront-message-broker repo don't use directly classes from magento/saas-export repo and vise-verse
    • Check composer.json dependencies
  • Legacy code is deleted

@mslabko
Copy link
Member

mslabko commented Dec 2, 2020

@magento run all tests

@mslabko mslabko mentioned this pull request Dec 2, 2020
Closed
7 tasks
@slavvka
Copy link
Member

slavvka commented Dec 2, 2020

@magento run all tests

mslabko
mslabko suggested changes Dec 2, 2020
View reviewed changes
app/code/Magento/ConfigurationImportMessageBroker/etc/module.xml Outdated
<config xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="urn:magento:framework:Module/etc/module.xsd">
<module name="Magento_ConfigurationImportMessageBroker">
<sequence>
<module name="Magento_MessageQueue" />
Copy link
Member

Choose a reason for hiding this comment

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

we can't depend on Magento module inside Message Broker

Copy link

Choose a reason for hiding this comment

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

removed

...agento/ConfigurationImportMessageBroker/Model/Configuration/Import/Mapping/SchemaLocator.php Outdated
*
* @var string
*/
protected $schema;
Copy link
Member

Choose a reason for hiding this comment

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

please, make it private here and in other places where it's possible

Copy link

Choose a reason for hiding this comment

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

replaced

mmansoor-magento pushed a commit that referenced this pull request Dec 3, 2020
@RuslanKostiv1
Merge pull request #4 from RuslanKostiv1/SFAPP-154-web-api-tests-fail
1a5ba38 
SFAPP-154: WEB-API test failures
@owlsmage
Copy link
Contributor Author

owlsmage commented Dec 4, 2020

@magento run Static Tests

@lykhachov
Copy link

@magento run Static Tests

@lykhachov
Copy link

@magento run Integration Tests

@lykhachov
Copy link

@magento run all tests

@mslabko
Copy link
Member

mslabko commented Dec 4, 2020 •
edited
Loading

Semantic Version Checker approved

MINOR | /etc/configuration_import_mapping.xsdMagento_ConfigurationImportMessageBroker:0

Integration tests is failed

Regarding error

Magento/ConfigurationImportMessageBroker/composer.json```

Please add mentioned module to the file, by analogy with

https://github.com/magento/storefront-search-ce/blob/b383042ccd1deaf56ca4415ec1e4a31c5448afa4/dev/tests/static/testsuite/Magento/Test/Integrity/_files/blacklist/composer_root_modules_storefront_search_ce.txt

@RuslanKostiv1
Copy link
Contributor

@owlsmage please add https://github.com/magento-commerce/magento2-infrastructure/pull/1395 in the list of Related Pull Requests

@RuslanKostiv1
Copy link
Contributor

@magento run Magento Health Index, Static Tests

1 similar comment
@RuslanKostiv1
Copy link
Contributor

@magento run Magento Health Index, Static Tests

@RuslanKostiv1
Copy link
Contributor

@magento run all tests

@RuslanKostiv1
Copy link
Contributor

@magento run all tests

@RuslanKostiv1
Copy link
Contributor

RuslanKostiv1 commented Dec 15, 2020 •
edited
Loading

@owlsmage @lykhachov there's static files failure:

Magento\Framework\Exception\LocalizedException: The following patterns didn't return any result:
app/code/Magento/CatalogMessageBroker/Model/ServiceConnector/Configuration/GrpcConfigurationProvider.php

https://public-results-storage-prod.magento-testing-service.engineering/reports/magento/storefront-message-broker/pull/4/423a2db3e3ddc9afc9bf482292be029e/Statics/console-error-logs.html

is it outdated information and dev/tests/static/testsuite/Magento/Test/Php/_files/phpstan/blacklist/storefront-message-broker.txt can be removed?

@RuslanKostiv1
Copy link
Contributor

@magento import pr to magento-commerce/storefront-message-broker

@magento-engcom-team
Copy link

@RuslanKostiv1 the pull request successfully imported.

@RuslanKostiv1
Copy link
Contributor

@magento run Static Tests

@mmansoor-magento mmansoor-magento merged commit cf3e13a into magento:main Dec 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lykhachov lykhachov lykhachov left review comments

@mslabko mslabko mslabko requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Configuration propagation
7 participants
@owlsmage @mslabko @slavvka @lykhachov @RuslanKostiv1 @magento-engcom-team @mmansoor-magento