Change some tests to run with restapi#1644
Change some tests to run with restapi#1644payalcha merged 19 commits intosecurefederatedai:developfrom
Conversation
Signed-off-by: Chaurasiya, Payal <payal.chaurasiya@intel.com>
Signed-off-by: Chaurasiya, Payal <payal.chaurasiya@intel.com>
Signed-off-by: Chaurasiya, Payal <payal.chaurasiya@intel.com>
Signed-off-by: Chaurasiya, Payal <payal.chaurasiya@intel.com>
Signed-off-by: Chaurasiya, Payal <payal.chaurasiya@intel.com>
|
Issue raised - #1646 for resiliency failure in Rest protocol. |
Signed-off-by: Chaurasiya, Payal <payal.chaurasiya@intel.com>
Signed-off-by: Chaurasiya, Payal <payal.chaurasiya@intel.com>
Signed-off-by: payalcha <payal.chaurasiya@intel.com>
Signed-off-by: payalcha <payal.chaurasiya@intel.com>
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the test suite by adding new REST protocol tests while migrating existing flags and configuration from “tr_rest_api” to “tr_rest_protocol”. Key changes include:
- Enforcing a model name check and passing the transport_protocol parameter in workspace, aggregator, and collaborator setups.
- Updating config flags and constants to support the new REST protocol.
- Adjusting workflow files to trigger tests with REST where applicable.
Reviewed Changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/end_to_end/utils/tr_workspace.py | Added model name check and propagated transport_protocol to aggregator/collaborator constructors. |
| tests/end_to_end/utils/federation_helper.py | Introduced protocol log checking and updated the setup_collaborator function. |
| tests/end_to_end/utils/db_helper.py | Increased retry count and sleep interval for database access in tests. |
| tests/end_to_end/utils/constants.py | Added TransportProtocol enum and removed duplicate KERAS_TENSORFLOW_MNIST constant. |
| tests/end_to_end/utils/conftest_helper.py | Renamed CLI flag to --tr_rest_protocol for clarity. |
| tests/end_to_end/models/model_owner.py | Updated test configuration to use the new REST protocol flag. |
| tests/end_to_end/models/collaborator.py | Modified constructor to require transport_protocol and updated related parameters. |
| tests/end_to_end/models/aggregator.py | Modified constructor to require transport_protocol and reduced check_sleep delay. |
| tests/end_to_end/conftest.py | Updated command-line argument to switch from tr_rest_api to tr_rest_protocol. |
| Various .github/workflows/*.yml | Adjusted test job names and parameters to leverage REST protocol testing. |
Comments suppressed due to low confidence (4)
tests/end_to_end/utils/conftest_helper.py:34
- The renaming from --tr_rest_api to --tr_rest_protocol improves clarity; ensure related documentation and usage are updated accordingly.
parser.add_argument("--tr_rest_protocol", action="store_true", help="Enable rest protocol in task runner. If not set, gRPC is used")
tests/end_to_end/models/collaborator.py:26
- Ensure that all instantiation calls for Collaborator are updated to provide the transport_protocol parameter, as it is now required.
def __init__(self, collaborator_name, transport_protocol, data_directory_path=None, workspace_path=None, container_id=None):
tests/end_to_end/models/aggregator.py:24
- Ensure that the transport_protocol parameter is consistently passed when creating an Aggregator instance, and that its value aligns with the updated TransportProtocol enum.
def __init__(self, agg_domain_name, workspace_path, transport_protocol, eval_scope=False, container_id=None):
tests/end_to_end/utils/constants.py:21
- Duplicate definition of KERAS_TENSORFLOW_MNIST was removed; verify that all references to this constant are correctly updated to avoid any inconsistency.
KERAS_TENSORFLOW_MNIST = "keras/tensorflow/mnist"
Signed-off-by: payalcha <payal.chaurasiya@intel.com>
There was a problem hiding this comment.
Pull Request Overview
Adds support for running existing and new end-to-end tests over the REST transport protocol in addition to gRPC.
- Extended workspace and plan setup helpers to accept a
transport_protocolparameter and updated log-based verification. - Propagated the new protocol flag (
--tr_rest_protocol/--transport-protocol) through test fixtures, collaborators, and aggregator classes. - Updated GitHub workflow definitions to enable REST-based test runs alongside or in place of gRPC.
Reviewed Changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/github/utils.py | Import YAML and update create_certified_workspace to set protocol |
| tests/github/test_hello_federation.py | Add --transport-protocol argument and pass through to setup |
| tests/end_to_end/utils/tr_workspace.py | Pass transport_protocol into aggregator and collaborator creation |
| tests/end_to_end/utils/federation_helper.py | Add _check_aggregator_protocol_log and set protocol from config |
| tests/end_to_end/utils/db_helper.py | Increase retries/sleep in get_key_value_from_db |
| tests/end_to_end/utils/constants.py | Introduce TransportProtocol enum and protocol constants |
| tests/end_to_end/utils/conftest_helper.py | Rename CLI flag to --tr_rest_protocol |
| tests/end_to_end/models/model_owner.py | Respect new tr_rest_protocol when modifying federation plan |
| tests/end_to_end/models/collaborator.py | Add transport_protocol field and reduce check_sleep durations |
| tests/end_to_end/models/aggregator.py | Add transport_protocol, reduce check_sleep |
| tests/end_to_end/conftest.py | Rename config attribute to tr_rest_protocol |
| .github/workflows/windows.yml | Add Windows job for REST-based Torch/MNIST run |
| .github/workflows/task_runner_straggler_e2e.yml | Bump Python version for straggler test |
| .github/workflows/task_runner_resiliency_e2e.yml | Split gRPC/REST resiliency jobs and update inputs |
| .github/workflows/task_runner_fedeval_e2e.yml | Rename jobs and flags for REST-based Fedeval scenarios |
| .github/workflows/task_runner_fed_analytics_e2e.yml | Update names and flags for REST in analytics tests |
| .github/workflows/task_runner_dockerized_ws_e2e.yml | Enable REST memory-logs job |
| .github/workflows/task_runner_connectivity_e2e.yml | Enable REST connectivity job |
| .github/workflows/pq_pipeline.yml | Wire up new connectivity and resiliency jobs in PQ pipeline |
Comments suppressed due to low confidence (1)
tests/end_to_end/utils/constants.py:77
- [nitpick] The constant name uses a lowercase 'g' in 'gRPC', which is inconsistent for acronyms. Consider renaming to
AGGREGATOR_GRPC_CLIENTto follow a consistent uppercase style.
AGGREGATOR_gRPC_CLIENT = "Starting Aggregator gRPC Server"
| plan_config['aggregator']['settings']['rounds_to_train'] = int(rounds_to_train) | ||
|
|
||
| # Update transport_protocol | ||
| plan_config['network']['settings']['transport_protocol'] = transport_protocol |
There was a problem hiding this comment.
[nitpick] The default protocol value is the literal 'grpc'. To reduce risk of typos and centralize valid options, consider using the TransportProtocol.GRPC.value enum member instead of duplicating the string.
Signed-off-by: payalcha <payal.chaurasiya@intel.com>
Signed-off-by: payalcha <payal.chaurasiya@intel.com>
* Change some tests to run with restapi Signed-off-by: Chaurasiya, Payal <payal.chaurasiya@intel.com> * Add grpc and rest checks Signed-off-by: Chaurasiya, Payal <payal.chaurasiya@intel.com> * small fix Signed-off-by: Chaurasiya, Payal <payal.chaurasiya@intel.com> * small fix Signed-off-by: Chaurasiya, Payal <payal.chaurasiya@intel.com> * add some fix Signed-off-by: Chaurasiya, Payal <payal.chaurasiya@intel.com> * rebase fix Signed-off-by: Chaurasiya, Payal <payal.chaurasiya@intel.com> * small fix Signed-off-by: Chaurasiya, Payal <payal.chaurasiya@intel.com> * commen resiliency Signed-off-by: Chaurasiya, Payal <payal.chaurasiya@intel.com> * Change rest_api to rest protocol Signed-off-by: Chaurasiya, Payal <payal.chaurasiya@intel.com> * Change rest_api to rest protocol Signed-off-by: Chaurasiya, Payal <payal.chaurasiya@intel.com> * Add one test in analytics Signed-off-by: payalcha <payal.chaurasiya@intel.com> * Add one docker test Signed-off-by: payalcha <payal.chaurasiya@intel.com> * Add one windows test Signed-off-by: payalcha <payal.chaurasiya@intel.com> * small fix Signed-off-by: payalcha <payal.chaurasiya@intel.com> * Small check Signed-off-by: payalcha <payal.chaurasiya@intel.com> * Small check Signed-off-by: payalcha <payal.chaurasiya@intel.com> --------- Signed-off-by: Chaurasiya, Payal <payal.chaurasiya@intel.com> Signed-off-by: payalcha <payal.chaurasiya@intel.com>
* Change some tests to run with restapi Signed-off-by: Chaurasiya, Payal <payal.chaurasiya@intel.com> * Add grpc and rest checks Signed-off-by: Chaurasiya, Payal <payal.chaurasiya@intel.com> * small fix Signed-off-by: Chaurasiya, Payal <payal.chaurasiya@intel.com> * small fix Signed-off-by: Chaurasiya, Payal <payal.chaurasiya@intel.com> * add some fix Signed-off-by: Chaurasiya, Payal <payal.chaurasiya@intel.com> * rebase fix Signed-off-by: Chaurasiya, Payal <payal.chaurasiya@intel.com> * small fix Signed-off-by: Chaurasiya, Payal <payal.chaurasiya@intel.com> * commen resiliency Signed-off-by: Chaurasiya, Payal <payal.chaurasiya@intel.com> * Change rest_api to rest protocol Signed-off-by: Chaurasiya, Payal <payal.chaurasiya@intel.com> * Change rest_api to rest protocol Signed-off-by: Chaurasiya, Payal <payal.chaurasiya@intel.com> * Add one test in analytics Signed-off-by: payalcha <payal.chaurasiya@intel.com> * Add one docker test Signed-off-by: payalcha <payal.chaurasiya@intel.com> * Add one windows test Signed-off-by: payalcha <payal.chaurasiya@intel.com> * small fix Signed-off-by: payalcha <payal.chaurasiya@intel.com> * Small check Signed-off-by: payalcha <payal.chaurasiya@intel.com> * Small check Signed-off-by: payalcha <payal.chaurasiya@intel.com> --------- Signed-off-by: Chaurasiya, Payal <payal.chaurasiya@intel.com> Signed-off-by: payalcha <payal.chaurasiya@intel.com> Signed-off-by: noopur <noopur@intel.com>
* Change some tests to run with restapi Signed-off-by: Chaurasiya, Payal <payal.chaurasiya@intel.com> * Add grpc and rest checks Signed-off-by: Chaurasiya, Payal <payal.chaurasiya@intel.com> * small fix Signed-off-by: Chaurasiya, Payal <payal.chaurasiya@intel.com> * small fix Signed-off-by: Chaurasiya, Payal <payal.chaurasiya@intel.com> * add some fix Signed-off-by: Chaurasiya, Payal <payal.chaurasiya@intel.com> * rebase fix Signed-off-by: Chaurasiya, Payal <payal.chaurasiya@intel.com> * small fix Signed-off-by: Chaurasiya, Payal <payal.chaurasiya@intel.com> * commen resiliency Signed-off-by: Chaurasiya, Payal <payal.chaurasiya@intel.com> * Change rest_api to rest protocol Signed-off-by: Chaurasiya, Payal <payal.chaurasiya@intel.com> * Change rest_api to rest protocol Signed-off-by: Chaurasiya, Payal <payal.chaurasiya@intel.com> * Add one test in analytics Signed-off-by: payalcha <payal.chaurasiya@intel.com> * Add one docker test Signed-off-by: payalcha <payal.chaurasiya@intel.com> * Add one windows test Signed-off-by: payalcha <payal.chaurasiya@intel.com> * small fix Signed-off-by: payalcha <payal.chaurasiya@intel.com> * Small check Signed-off-by: payalcha <payal.chaurasiya@intel.com> * Small check Signed-off-by: payalcha <payal.chaurasiya@intel.com> --------- Signed-off-by: Chaurasiya, Payal <payal.chaurasiya@intel.com> Signed-off-by: payalcha <payal.chaurasiya@intel.com>
Summary
Add tests to check rest protocol.
Type of Change (Mandatory)
Specify the type of change being made.
Description (Mandatory)
Testing
PR and PQ pipeline
Please note any existing test can run with "rest" by just passing --tr_rest_protocol flag.