Implement Monitor and MonitoringThreadContainer #58
Conversation
| @@ -11,13 +11,3 @@ | |||
| # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | |||
| # See the License for the specific language governing permissions and | |||
| # limitations under the License. | |||
There was a problem hiding this comment.
Not sure what happened with git here, but I deleted subscribed_method_utils.py and moved the SubscribedMethodUtils class to aws_wrapper/utils/utils.py instead, then added this separate init.py file to unit_testing/utils/
There was a problem hiding this comment.
This is weird. Do we really need this file at all?
There was a problem hiding this comment.
Good question, according to this source init.py used to be required to mark packages as python packages, even if it was empty. But starting with Python 3.3 they aren't required anymore. I'll leave this in since we have blank ones in most/all our packages but we could consider creating a PR to remove the empty ones
congoamz
requested review from
karenc-bq,
justing-bq and
joyc-bq
and removed request for
karenc-bq
| from threading import Lock | ||
|
|
||
|
|
||
| class AtomicInt: |
There was a problem hiding this comment.
Does there already exist a library that can do this for us?
There was a problem hiding this comment.
Looked around, found this. Looks like you can use pypy but it looks like pypy is a different implementation of python instead of just a library. Not sure if we want to switch to get their implementation. Maybe we could create another task to investigate if it is viable or if there is a different library out there
| @@ -11,13 +11,3 @@ | |||
| # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | |||
| # See the License for the specific language governing permissions and | |||
| # limitations under the License. | |||
There was a problem hiding this comment.
This is weird. Do we really need this file at all?
| delay_ms = Monitor._INACTIVE_SLEEP_MS | ||
| else: | ||
| # Subtract the time taken for the status check from the delay | ||
| delay_ms -= status.elapsed_time_ns |
There was a problem hiding this comment.
Does it makes sense to minus nanoseconds from milliseconds? Wouldn't we have something like 500 ms - 100,000,000,000 ns?
There was a problem hiding this comment.
Good catch. The JDBC code does not convert either so we'll need to fix it there
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.