Staging #103
Staging #103
Conversation
…hentication still throws and exception and immediately fails)
…timings. Also delete unwanted WAV file
…fix for time-offset for restarts
Better random filename
Add entities and endpoints for EPubs and Images
…dias Filter out null medias for GetAllWatchedMediaForUser
…stranscribe/WebAPI into DupTranscriptionRestarts-99
Dup transcription restarts 99
| string[] keyregion = subscriptionKey.Split(','); | ||
| if (keyregion.Length != 2) | ||
| { | ||
| throw new Exception("AZURE_SUBSCRIPTION_KEYS should be in the form key,region;key,region"); |
There was a problem hiding this comment.
can we add the offending line here in the log message.
There was a problem hiding this comment.
There was no logger in this class! Will fix though in a future commit
| foreach (string subscriptionKey in subscriptionKeys.Split(';')) | ||
| { | ||
| if (!subscriptionKeys.Contains(",")) | ||
| { | ||
| continue; |
There was a problem hiding this comment.
maybe logging.debug, so we can see something got ignored.
| }; | ||
| _logger.LogInformation($"Connection to RabbitMQ server {factory.HostName} with user {factory.UserName} on port {factory.Port}..."); |
There was a problem hiding this comment.
comment, not sure if this is an issue;
1 connection per application, this is long lived
1 channel per thread, from the documentation "Channel instances must not be shared by threads that publish on them."
There was a problem hiding this comment.
Made a new issue--
#104
| _logger.LogInformation(" [*] Waiting for messages, queueName - {0}", queueName); | ||
|
|
||
| var consumer = new EventingBasicConsumer(_channel); | ||
| consumer.Received += async (model, ea) => | ||
| { | ||
| // This object exists so that we can wrap all OnConsumes with a try-finally here |
There was a problem hiding this comment.
One problem I have seen in the past with RabbitMQ, if your job takes to long to execute, the heartbeat will timeout (that is done inside the BasicConsume function (I believe, in my case it was python code). This would result in closed connections.
There was a problem hiding this comment.
So what do you recommend?
There was a problem hiding this comment.
what we did was complex, not sure it is the best yet:
worker = None
while(consumeMessage(timeout = 100ms)) {
if worker and worker.finished:
channel.ack
if newjob and worker = None
worker = new Job(finished=False)
}
Job() {
finished = False
run():
do some work
finished=True
you consume a message, start a new thread, go back to consumeMessage so you can send heartbeat. When job is finished Ack the message and get a new message. If you have. a list of workers, you might be able to handle multiple in parallel (just need to push the msg to the worker so you know what job to ack).
| var configurations = CTDbContext.GetConfigurations(); | ||
| return "Server=" + configurations["POSTGRES_SERVER_NAME"] + ";Port=5432;Database=" | ||
| + configurations["POSTGRES_DB"] + ";User Id=" + configurations["ADMIN_USER_ID"] + ";Password=" + configurations["ADMIN_PASSWORD"] + ";MaxPoolSize=1000;"; | ||
| return "Server=" + configurations["POSTGRES_SERVER_NAME"] |
There was a problem hiding this comment.
Since we are building the URI here, can we not just pass in the URI as an argument. That way we can add MaxPoolSize as part of the configuration and there is no need to recompile the code.
| @@ -17,6 +18,9 @@ public class AppSettings | |||
| public string ADMIN_USER_ID { get; set; } | |||
| public string ADMIN_PASSWORD { get; set; } | |||
| public string RabbitMQServer { get; set; } | |||
|
|
|||
There was a problem hiding this comment.
we should be consistent with variables like this. I think it be good to look at a linter that checks formatting (yes I care about that).
See
- https://docs.microsoft.com/en-us/dotnet/csharp/programming-guide/inside-a-program/coding-conventions
- https://google.github.io/styleguide/csharp-style.html
Might be interesting to look at:
https://marketplace.visualstudio.com/items?itemName=ChrisDahlberg.StyleCop
staging to master