164 log the topic #177
Merged
164 log the topic #177
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Attempts to unit test the logging proved to be quite difficult, and all though it would be nice to have, since this is a small change I don't think its necessary to add. |
|
New logs based on updated comments from issue #164, the logging messages are the same as is seen with STOMP, however, no port is logged as it is not required for AMS see: Lines 148 to 152 in db3dace Sender log: Receiver log: Also note that the ams package does not have a '.__version__' attribute associated with it, therefore pkg_resources.get_distribution() package has been imported and utilised. |
tofu-rocketry
previously requested changes
Dec 2, 2021
tofu-rocketry
force-pushed
the
164-log-the-topic
branch
from
02a90d5 to
80b2f8f
Compare
tofu-rocketry
force-pushed
the
164-log-the-topic
branch
2 times, most recently
from
7240e1b to
b76fc05
Compare
Signed-off-by: jounaidr <[email protected]>
Signed-off-by: jounaidr <[email protected]>
This seperates out a helper method, and adds said method to run_reveiver and run_sender methods in agents.py Signed-off-by: jounaidr <[email protected]>
Signed-off-by: jounaidr <[email protected]>
Add extra logging for AMS including version, host and port, and destination queue (as is with STOMP). Signed-off-by: jounaidr <[email protected]>
Signed-off-by: jounaidr <[email protected]>
- Version only requires 1 string argument as it's taken using pkg_resources.get_distribution - Port is omitted as it's not required for AMS Signed-off-by: jounaidr <[email protected]>
With method contents only in run_sender(), as since the logging is done elsewhere, this change is out of scope for this issue (although it would probably be better still to have). Signed-off-by: jounaidr <[email protected]>
Signed-off-by: jounaidr <[email protected]>
As it's only to enhance the logging, we'll just log the version of argo_ams_library if the module is available. pkg_resources is distributed with setuptools, which is commonly installed, but we don't want to add any dependencies for this "nice to have" functionality.
tofu-rocketry
force-pushed
the
164-log-the-topic
branch
from
6260529 to
b748b04
Compare
tofu-rocketry
force-pushed
the
164-log-the-topic
branch
from
b748b04 to
b9a2b60
Compare
This reverts commit 46756d1. We don't need to make the use of pkg_resources optional as setuptools (which includes it) is already a dependency of python-ldap.
We use pkg_resources with is distributed with setuptools. Even though setuptools is a dependency of python-ldap, so it would get installed during dependency resolution, it's best to be explicit.
argo-ams-library is no longer experimental
tofu-rocketry
force-pushed
the
164-log-the-topic
branch
from
b9a2b60 to
d9711a3
Compare
|
More testing (ignore pidfile warning):
|
tofu-rocketry
approved these changes
Aug 3, 2022
Letizia97
approved these changes
Aug 8, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Resolves #164
Log output for sender.py:
Log output for receiver.py:
Note that for testing purposes a new certificate was used, which is what's causing the error seen in the above logs.