AIT-Core icon indicating copy to clipboard operation
AIT-Core copied to clipboard

Issue #517 - TCP Input Support

Open cjjacks opened this issue 1 year ago • 6 comments

  • Adds a TCP client and server option to input streams
  • Updates docunmentation with new input specifications
  • Adds client specific tests
  • Adds additional stream tests
  • Formatting

cjjacks avatar Mar 14 '24 16:03 cjjacks

Hopefully I've got this PR set up right - I'm more used to Gitlab which has slightly different mechanics. Also, I apologize for the large number of changes - a large portion of the changes are just formatting updates from merging #519.

cjjacks avatar Mar 14 '24 16:03 cjjacks

Sorry for the delay in getting this ready to review. I got side tracked with some other projects. Here is a summary of what I have done in this pr:

- Adds a TCP client and server option to input streams
- Adds option to use 0.0.0.0 for server binding
- Adds TCP to output clients and the ability to send to a remote host
- Adds factory methods to determine the correct stream to instantiate based on config
- Updates documentation with new input/output specifications
- Adds client specific tests
- Adds additional stream tests
- Repo wide formatting from bringing in pre-commit changes
- Temporarily disables python 3.7 tests
- add a docker compose stack to test network configs

Also, sorry for the large amount of changes in this PR. The vast majority of changes are just auto formatting done by running:

pre-commit run -a

This brings the enitre repo in line with the linter and not just the diff. If its too overwhelming i can back these changes out of the PR.

As far as verification, I have added some tests as well as docker compose stack to test the different clients/servers. If you have make, docker and docker compose. you can run the following:

make network-test

and this will do some stress testing with all the new input/output stream features. Additionally, I have been using this branch and specifically the TCPInputStream for some time now as part of developing with AIT Core

cjjacks avatar Jul 26 '24 21:07 cjjacks

@cjjacks Sorry for our extnsive delay, our time has been limited and what little there is has been spent investigating separate security issues.

So some notes thus far (with requested changes):

  1. Linter updates: Looks like we would prefer for them to be their own PR, preferably before the TCP changes. So could you please separate those?

  2. New constants: ait.core.constants should be in ait.core.init with all the other constants. Or, at the very least, all the constants should get moved into constants.py and then exposed in the init.py file for consistency

  3. “Address specification”: the syntax breaks the existing convention so could this be revisited. At the very least,

            - "UDP"
            - "server"
            - 3077

would be better as 'UDP:server:3077’, or something similar.

  1. The Docker/makefile may need to be their own PRs, so that the TCP update is better isolated

  2. OutputClient: Currently it is supporting both UDP and TCP cases. Would there be a way to cleanly separate these into separate classes, and update 'PortOutputStream' extensions accordingly?

Thanks, Nicko

nttoole avatar Sep 17 '24 23:09 nttoole

Quality Gate Failed Quality Gate failed

Failed conditions
2 Security Hotspots
E Reliability Rating on New Code (required ≥ A)

See analysis details on SonarCloud

Catch issues before they fail your Quality Gate with our IDE extension SonarLint

sonarqubecloud[bot] avatar Sep 18 '24 22:09 sonarqubecloud[bot]

This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation.

Sorry for the delay again. I'll get to work addressing the feedback above!

cjjacks avatar Sep 26 '24 17:09 cjjacks