AMQ adapter version w/o endpoints for CleverSwarm integration #3

Merged
stanislav.hejny merged 12 commits from feat/#1 into develop 2025-02-28 12:03:53 +00:00
stanislav.hejny commented 2025-01-23 10:28:46 +00:00 (Migrated from git.cleverthis.com)

Ref epic: CleverSwarm Onboarding #58

this implements #1

Ref epic: [CleverSwarm Onboarding #58](https://git.cleverthis.com/clevermicro/clevermicro/issues/58) this implements #1
hurui200320 (Migrated from git.cleverthis.com) approved these changes 2025-01-23 10:28:46 +00:00
abed.alrahman (Migrated from git.cleverthis.com) approved these changes 2025-01-23 10:28:46 +00:00
stanislav.hejny commented 2025-01-23 10:28:47 +00:00 (Migrated from git.cleverthis.com)

requested review from @abed.alrahman and @hurui200320

requested review from @abed.alrahman and @hurui200320
stanislav.hejny commented 2025-01-23 10:28:47 +00:00 (Migrated from git.cleverthis.com)

assigned to @stanislav.hejny

assigned to @stanislav.hejny
hurui200320 commented 2025-01-23 11:23:29 +00:00 (Migrated from git.cleverthis.com)

In this case, we can accept both str and bytes-like object for messages. I think we can mark the type of message as str | bytes (I'd prefer immutable type bytes instead of mutable bytearray, and other bytes-like object types don't even make sense here). Although it doesn't impact anything during the runtime, it does help on static analysis (if we have one) when someone tries to give an int number, since I assume the base64.b64encode doesn't accept int objects.

Ref: https://docs.python.org/3/library/base64.html#base64.b64encode
Ref: https://docs.python.org/3/glossary.html#term-bytes-like-object

Also something like Javadoc (but for python, I don't know what it's called) might be helpful to tell the user (presumably Luis) which type you can pass in.

In this case, we can accept both str and bytes-like object for messages. I think we can mark the type of `message` as `str | bytes` (I'd prefer immutable type `bytes` instead of mutable `bytearray`, and other bytes-like object types don't even make sense here). Although it doesn't impact anything during the runtime, it does help on static analysis (if we have one) when someone tries to give an int number, since I assume the `base64.b64encode` doesn't accept int objects. > Ref: https://docs.python.org/3/library/base64.html#base64.b64encode > Ref: https://docs.python.org/3/glossary.html#term-bytes-like-object Also something like Javadoc (but for python, I don't know what it's called) might be helpful to tell the user (presumably Luis) which type you can pass in.
hurui200320 commented 2025-01-23 11:23:29 +00:00 (Migrated from git.cleverthis.com)

If this adaptor is specifically made for CleverSwarm, should it be part of the CleverSwarm code base, instead of living in this repo as a public library? It's ok if it's temporarily living there for the integration since it's time-limited, but long term wise I think it's better moved to CleverSwarm code base.

If this adaptor is specifically made for CleverSwarm, should it be part of the CleverSwarm code base, instead of living in this repo as a public library? It's ok if it's temporarily living there for the integration since it's time-limited, but long term wise I think it's better moved to CleverSwarm code base.
hurui200320 commented 2025-01-23 11:23:29 +00:00 (Migrated from git.cleverthis.com)

It's not good to hardcode such logic in the code. Generally I would recommend using a cmdline option or an env variable for this. For example, FOO_CONFIG_PATH (replace the FOO with library name, using it as a namespace). If the env is not provided or the value is not pointing to a valid file, then you can fall back on default values like ~/.config/foo/application.properties (using the home folder of the user, which should be available on both linux and windows).

It's not good to hardcode such logic in the code. Generally I would recommend using a cmdline option or an env variable for this. For example, `FOO_CONFIG_PATH` (replace the `FOO` with library name, using it as a namespace). If the env is not provided or the value is not pointing to a valid file, then you can fall back on default values like `~/.config/foo/application.properties` (using the home folder of the user, which should be available on both linux and windows).
hurui200320 commented 2025-01-23 11:23:29 +00:00 (Migrated from git.cleverthis.com)

I suggest we put the UNKNOWN at the beginning, so that in the future we don't have cases like REQUEST_FOO=1, UNKNOWN=2 and REQUEST_BAR=3 when adding new items. This is more like a style thing.

I suggest we put the `UNKNOWN` at the beginning, so that in the future we don't have cases like `REQUEST_FOO=1`, `UNKNOWN=2` and `REQUEST_BAR=3` when adding new items. This is more like a style thing.
hurui200320 commented 2025-01-23 11:23:30 +00:00 (Migrated from git.cleverthis.com)

These files are no longer needed since they don't work with docker swarm. I would suggest we get rid of those files before merging. But if you need them for testing and debugging, feel free to ignore this comment.

These files are no longer needed since they don't work with docker swarm. I would suggest we get rid of those files before merging. But if you need them for testing and debugging, feel free to ignore this comment.
hurui200320 commented 2025-01-23 11:23:30 +00:00 (Migrated from git.cleverthis.com)

Thank you for your work. I'm not very familiar with python projects, so I might not be able to catch some specific bugs. Currently I did a line-by-line review and don't see any problems. All my comments are more like a suggestion, so feel free to ignore them if you have a reason to do so or I'm wrong.

If you need more professional opinion on the code, I suggest you ask Jeff or people from other teams who know python and related libraries.

Thank you for your work. I'm not very familiar with python projects, so I might not be able to catch some specific bugs. Currently I did a line-by-line review and don't see any problems. All my comments are more like a suggestion, so feel free to ignore them if you have a reason to do so or I'm wrong. If you need more professional opinion on the code, I suggest you ask Jeff or people from other teams who know python and related libraries.
hurui200320 commented 2025-01-23 11:23:30 +00:00 (Migrated from git.cleverthis.com)

requested changes

requested changes
stanislav.hejny commented 2025-01-23 11:48:25 +00:00 (Migrated from git.cleverthis.com)

U'r right, this is a shortcut, because the final version is still dependent on final integration effort. The problem is that I need a path to file from CWD (or absolute path) and I don't know what it would be after integration. Even the cmd-line parameter has to be decided with CleveSwarm. this should be perhaps marked as TODO or better documented

U'r right, this is a shortcut, because the final version is still dependent on final integration effort. The problem is that I need a path to file from CWD (or absolute path) and I don't know what it would be after integration. Even the cmd-line parameter has to be decided with CleveSwarm. this should be perhaps marked as TODO or better documented
stanislav.hejny commented 2025-01-23 11:56:07 +00:00 (Migrated from git.cleverthis.com)

I will change that in later version (v0.2), because in Python, enum ordinals start at 1, while in Java enum ordinals start at 0. I need to adjust the same enums on Java side so that the ordinal values are matching (at this moment I add 1 when decoding the message produced by Java based adapter).

I will change that in later version (v0.2), because in Python, enum ordinals start at 1, while in Java enum ordinals start at 0. I need to adjust the same enums on Java side so that the ordinal values are matching (at this moment I add 1 when decoding the message produced by Java based adapter).
stanislav.hejny commented 2025-01-23 11:58:13 +00:00 (Migrated from git.cleverthis.com)

I perhaps don't need them all, it was just the docker compose won't work w/o it. But indeed those are only for the local testing purposes.

I perhaps don't need them all, it was just the docker compose won't work w/o it. But indeed those are only for the local testing purposes.
hurui200320 commented 2025-01-23 12:19:42 +00:00 (Migrated from git.cleverthis.com)

Ok, in that case you can add a TODO in the code and mark this thread as resolved.

Ok, in that case you can add a TODO in the code and mark this thread as resolved.
hurui200320 commented 2025-02-20 07:47:27 +00:00 (Migrated from git.cleverthis.com)

added 5 commits

  • 635b1003 - Initial commit
  • 0bbd8e29 - Added Apache OSS license
  • 6f35b5e4 - Initial commit
  • d2b1b59c - Converted AMQPAdapeter Java code to Python
  • 286a4986 - AMQ adapter version w/o endpints for CleverSwarm integration

Compare with previous version

added 5 commits <ul><li>635b1003 - Initial commit</li><li>0bbd8e29 - Added Apache OSS license</li><li>6f35b5e4 - Initial commit</li><li>d2b1b59c - Converted AMQPAdapeter Java code to Python</li><li>286a4986 - AMQ adapter version w/o endpints for CleverSwarm integration</li></ul> [Compare with previous version](/cleverthis/clevermicro/amq-adapter-python/-/merge_requests/1/diffs?diff_id=2375&start_sha=856e662e47814b4826423cbba58504056c6d8bc1)
stanislav.hejny commented 2025-02-25 20:37:51 +00:00 (Migrated from git.cleverthis.com)

changed this line in version 4 of the diff

changed this line in [version 4 of the diff](/cleverthis/clevermicro/amq-adapter-python/-/merge_requests/1/diffs?diff_id=2414&start_sha=286a4986b32c8e23af44816a0c5520188365497d#6899651d5a86ae012a1890cc5bf37168b8188966_79_96)
stanislav.hejny commented 2025-02-25 20:37:53 +00:00 (Migrated from git.cleverthis.com)

added 1 commit

  • eb26eec7 - CleverSwarm API invocation example

Compare with previous version

added 1 commit <ul><li>eb26eec7 - CleverSwarm API invocation example</li></ul> [Compare with previous version](/cleverthis/clevermicro/amq-adapter-python/-/merge_requests/1/diffs?diff_id=2414&start_sha=286a4986b32c8e23af44816a0c5520188365497d)
stanislav.hejny commented 2025-02-26 07:42:09 +00:00 (Migrated from git.cleverthis.com)

Changed to be bytes only

Changed to be bytes only
stanislav.hejny commented 2025-02-26 07:44:46 +00:00 (Migrated from git.cleverthis.com)

This entire python code is to be included to CleverSwarm, as library, perhaps, but we start by giving CS everything. Still need to work out how to make it more generic

This entire python code is to be included to CleverSwarm, as library, perhaps, but we start by giving CS everything. Still need to work out how to make it more generic
stanislav.hejny commented 2025-02-26 07:45:57 +00:00 (Migrated from git.cleverthis.com)

resolved all threads

resolved all threads
stanislav.hejny commented 2025-02-26 10:51:44 +00:00 (Migrated from git.cleverthis.com)

changed this line in version 5 of the diff

changed this line in [version 5 of the diff](/cleverthis/clevermicro/amq-adapter-python/-/merge_requests/1/diffs?diff_id=2423&start_sha=eb26eec7674f9d389ac2bd7e2a04955a8b4c5875#7d26d7962d41f49f6500756478ee45c012e0e436_200_0)
stanislav.hejny commented 2025-02-26 10:51:45 +00:00 (Migrated from git.cleverthis.com)

added 1 commit

  • 1f904072 - feat/#1 - refactor code to allow make it as library and to separate the AMQ...

Compare with previous version

added 1 commit <ul><li>1f904072 - feat/#1 - refactor code to allow make it as library and to separate the AMQ...</li></ul> [Compare with previous version](/cleverthis/clevermicro/amq-adapter-python/-/merge_requests/1/diffs?diff_id=2423&start_sha=eb26eec7674f9d389ac2bd7e2a04955a8b4c5875)
stanislav.hejny commented 2025-02-26 19:54:04 +00:00 (Migrated from git.cleverthis.com)

added 1 commit

  • 73c962dc - Add CI/CD pipeline, adjust for integration with CleverSwarm and other python services

Compare with previous version

added 1 commit <ul><li>73c962dc - Add CI/CD pipeline, adjust for integration with CleverSwarm and other python services</li></ul> [Compare with previous version](/cleverthis/clevermicro/amq-adapter-python/-/merge_requests/1/diffs?diff_id=2428&start_sha=1f9040729f4a68feb5e69ffb0e9b64db8ffab6bf)
stanislav.hejny commented 2025-02-26 19:55:49 +00:00 (Migrated from git.cleverthis.com)

enabled automatic add to merge train when checks pass

enabled automatic add to merge train when checks pass
stanislav.hejny commented 2025-02-26 20:30:31 +00:00 (Migrated from git.cleverthis.com)

aborted automatic add to merge train because the source branch was updated. Learn more.

aborted automatic add to merge train because the source branch was updated. [Learn more](https://git.cleverthis.com/help/ci/pipelines/merge_trains.md#merge-request-dropped-from-the-merge-train).
stanislav.hejny commented 2025-02-26 20:30:31 +00:00 (Migrated from git.cleverthis.com)

added 1 commit

  • 1b042196 - feat/#1 - add the code coverage run to the pipeline

Compare with previous version

added 1 commit <ul><li>1b042196 - feat/#1 - add the code coverage run to the pipeline</li></ul> [Compare with previous version](/cleverthis/clevermicro/amq-adapter-python/-/merge_requests/1/diffs?diff_id=2430&start_sha=73c962dc12f26ef6afd840080c9c70342048ca8b)
stanislav.hejny commented 2025-02-26 22:37:56 +00:00 (Migrated from git.cleverthis.com)

enabled automatic add to merge train when checks pass

enabled automatic add to merge train when checks pass
stanislav.hejny commented 2025-02-26 22:39:50 +00:00 (Migrated from git.cleverthis.com)

bypassed reviews on this merge request

bypassed reviews on this merge request
stanislav.hejny commented 2025-02-27 03:57:35 +00:00 (Migrated from git.cleverthis.com)

aborted automatic add to merge train because the source branch was updated. Learn more.

aborted automatic add to merge train because the source branch was updated. [Learn more](https://git.cleverthis.com/help/ci/pipelines/merge_trains.md#merge-request-dropped-from-the-merge-train).
stanislav.hejny commented 2025-02-27 03:57:35 +00:00 (Migrated from git.cleverthis.com)

added 1 commit

  • a633ecef - Update the Ci/CD pipline to publish AMQ as library

Compare with previous version

added 1 commit <ul><li>a633ecef - Update the Ci/CD pipline to publish AMQ as library</li></ul> [Compare with previous version](/cleverthis/clevermicro/amq-adapter-python/-/merge_requests/1/diffs?diff_id=2434&start_sha=1b0421964c5230fa71f279c73c737fa5c19cc536)
stanislav.hejny commented 2025-02-27 04:11:20 +00:00 (Migrated from git.cleverthis.com)

added 1 commit

  • 30e046b2 - Update the Ci/CD pipline to publish AMQ as library

Compare with previous version

added 1 commit <ul><li>30e046b2 - Update the Ci/CD pipline to publish AMQ as library</li></ul> [Compare with previous version](/cleverthis/clevermicro/amq-adapter-python/-/merge_requests/1/diffs?diff_id=2436&start_sha=a633ecef4cb8ce1fe35e9895eac314aeef79fd21)
stanislav.hejny commented 2025-02-27 08:55:47 +00:00 (Migrated from git.cleverthis.com)

requested review from @hurui200320

requested review from @hurui200320
hurui200320 commented 2025-02-27 09:31:48 +00:00 (Migrated from git.cleverthis.com)

mentioned in merge request !4

mentioned in merge request !4
hurui200320 commented 2025-02-27 09:36:19 +00:00 (Migrated from git.cleverthis.com)

approved this merge request

approved this merge request
abed.alrahman commented 2025-02-27 09:56:53 +00:00 (Migrated from git.cleverthis.com)

approved this merge request

approved this merge request
stanislav.hejny commented 2025-02-27 21:44:36 +00:00 (Migrated from git.cleverthis.com)

added 1 commit

  • e861bf0f - Remove references to cleverswarm and fix packaging

Compare with previous version

added 1 commit <ul><li>e861bf0f - Remove references to cleverswarm and fix packaging</li></ul> [Compare with previous version](/cleverthis/clevermicro/amq-adapter-python/-/merge_requests/1/diffs?diff_id=2458&start_sha=30e046b23439a5b4c52034f6f3462dcedad5f658)
stanislav.hejny commented 2025-02-27 21:44:49 +00:00 (Migrated from git.cleverthis.com)

reset approvals from @abed.alrahman and @hurui200320 by pushing to the branch

reset approvals from @abed.alrahman and @hurui200320 by pushing to the branch
stanislav.hejny commented 2025-02-27 21:50:55 +00:00 (Migrated from git.cleverthis.com)

added 1 commit

  • dc884c65 - Bump-up project version to enable publish to PyPI

Compare with previous version

added 1 commit <ul><li>dc884c65 - Bump-up project version to enable publish to PyPI</li></ul> [Compare with previous version](/cleverthis/clevermicro/amq-adapter-python/-/merge_requests/1/diffs?diff_id=2460&start_sha=e861bf0f38d1c2ebae2d3770fe16b31d2236a3fd)
stanislav.hejny commented 2025-02-27 23:08:37 +00:00 (Migrated from git.cleverthis.com)

added 1 commit

  • aaf1c77e - Fix cleverswarm dependencies identified in testing

Compare with previous version

added 1 commit <ul><li>aaf1c77e - Fix cleverswarm dependencies identified in testing</li></ul> [Compare with previous version](/cleverthis/clevermicro/amq-adapter-python/-/merge_requests/1/diffs?diff_id=2462&start_sha=dc884c654a3d2fc5894ff8733fac37fe056ed506)
stanislav.hejny commented 2025-02-28 00:10:35 +00:00 (Migrated from git.cleverthis.com)

added 1 commit

  • aaa2895c - remove hardcoded config file path

Compare with previous version

added 1 commit <ul><li>aaa2895c - remove hardcoded config file path</li></ul> [Compare with previous version](/cleverthis/clevermicro/amq-adapter-python/-/merge_requests/1/diffs?diff_id=2464&start_sha=aaf1c77e72160f8f1e0dd73acabec26307ec8629)
stanislav.hejny commented 2025-02-28 00:15:10 +00:00 (Migrated from git.cleverthis.com)

added 1 commit

  • d9af0d09 - Bump-up project version to enable publish to PyPI

Compare with previous version

added 1 commit <ul><li>d9af0d09 - Bump-up project version to enable publish to PyPI</li></ul> [Compare with previous version](/cleverthis/clevermicro/amq-adapter-python/-/merge_requests/1/diffs?diff_id=2466&start_sha=aaa2895cbcd3abf5abd63ad31471193fca73c5a0)
hurui200320 commented 2025-02-28 10:50:48 +00:00 (Migrated from git.cleverthis.com)

approved this merge request

approved this merge request
abed.alrahman commented 2025-02-28 11:03:04 +00:00 (Migrated from git.cleverthis.com)

approved this merge request

approved this merge request
stanislav.hejny commented 2025-02-28 12:03:31 +00:00 (Migrated from git.cleverthis.com)

started a merge train

started a [merge train](https://git.cleverthis.com/cleverthis/clevermicro/amq-adapter-python/-/merge_trains)
stanislav.hejny commented 2025-02-28 12:03:35 +00:00 (Migrated from git.cleverthis.com)

removed this merge request from the merge train because The resulting pipeline would have been empty. Review the rules configuration for the relevant jobs.

removed this merge request from the merge train because The resulting pipeline would have been empty. Review the rules configuration for the relevant jobs.
stanislav.hejny commented 2025-02-28 12:03:54 +00:00 (Migrated from git.cleverthis.com)

mentioned in commit e8f4abdfaf

mentioned in commit e8f4abdfaf81a856980491c0cb31de5713e44bf5
stanislav.hejny (Migrated from git.cleverthis.com) merged commit e8f4abdfaf into develop 2025-02-28 12:03:56 +00:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: clevermicro/amq-adapter-python#3
No description provided.