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

Multi destination #26

Closed
wants to merge 16 commits into from
Closed

Multi destination #26

wants to merge 16 commits into from

Conversation

jhlee-mitre
Copy link
Contributor

Implemented a new feature to handle multi-destination in TestScript.

Detail:

  1. In driver.rb changed endpoint to Array to store multiple server addresses. I put seven predefined, different server addresses from HAPI, Firely, and Logica. We need to start discussion of how to deal with multiple server addresses. Questions may include 1) when we have multiple destinations, should they be always different servers or server instances? 2) What if 100 multiple destinations in TestScript? Should the engine get prepared for 100 server addresses? 3) Do we keep server address in hard coded way as it is, or would we create a configuration file for the engine (e.g. configure.xml)?

  2. Client object corresponds to destination. If there are two destinations in TestScript, the engine instantiates two Clients. If a TestScript doesn't include destination, the engine considers to use "default" client, which picks the first one from the server address list in endpoints.

  3. Client object is moved from TestScriptEngine.rb to TestScriptRunnable.rb, since now one client won't be used by multiple runnables. Each runnable should have own instance of Client correspond to destination.

  4. The engine reads multiple origins but doesn't do anything with it for now.

Additional:

  1. Example TestScript for multi-agent is added.

@jawalonoski
Copy link
Member

Hard-coding the server list doesn't make sense to me. Since it is a command-line tool, I would expect the user to list the servers they want to use as arguments. Acceptable alternatives include writing a shell script for frequently used scenarios or creating a configuration file.

The user wants to run a TestScript with 2 destinations? Then they are required to provide two URLs.

The engine shouldn't run if an endpoint is not specified.

It doesn't matter if the array is size 1 or size 1 billion. Other than memory. And you'd need a scenario that has that many endpoints. But it shouldn't matter.

@jhlee-mitre
Copy link
Contributor Author

  1. If no server address entered in argument, default server is assigned.
  2. If one server address entered in argument, it replaces default server.
  3. If there are destination in TestScript, servers are assigned to destinations in order in argument
  4. If # of server address < destination, the engine errors.

Test with these:

ruby driver.rb read_test_script.json  
ruby driver.rb http:https://hapi.fhir.org/baseR4 read_test_script.json
ruby driver.rb http:https://hapi.fhir.org/baseR4 multisystem_test_script.json
ruby driver.rb http:https://hapi.fhir.org/baseR4 https://server.fire.ly/ multisystem_test_script.json
ruby driver.rb http:https://hapi.fhir.org/baseR4 https://server.fire.ly/ read_test_script.json

Copy link
Contributor

@ms-k1ngk0ng ms-k1ngk0ng left a comment

Choose a reason for hiding this comment

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

Nice! I think the biggest change needed is how/when we prompt users for endpoints. I've included my thoughts, but if any of it is unclear, let me know and we can discuss.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
lib/TestScriptEngine.rb Show resolved Hide resolved
lib/TestScriptRunnable.rb Show resolved Hide resolved
lib/TestScriptRunnable.rb Show resolved Hide resolved
lib/TestScriptRunnable.rb Show resolved Hide resolved
lib/TestScriptRunnable.rb Outdated Show resolved Hide resolved
lib/driver.rb Show resolved Hide resolved
@jhlee-mitre jhlee-mitre linked an issue Oct 24, 2022 that may be closed by this pull request
@jhlee-mitre jhlee-mitre linked an issue Nov 28, 2022 that may be closed by this pull request
@jhlee-mitre jhlee-mitre changed the title Add multi destination Multi destination Dec 12, 2022
@jhlee-mitre jhlee-mitre removed a link to an issue Dec 30, 2022
@jhlee-mitre
Copy link
Contributor Author

jhlee-mitre commented Dec 30, 2022

Close this PR by end of the project. Issue #53 will keep open without milestone.

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.

Client Class
3 participants