-
Notifications
You must be signed in to change notification settings - Fork 8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update CosmosDB Samples #141
base: main
Are you sure you want to change the base?
Conversation
|
||
namespace Microsoft.Azure.WebJobs.Extensions.Redis.Samples.CosmosDB.Models | ||
{ | ||
public class StreamData |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this not a record
like PubSubData
and RedisData
?
// Helper method to format stream message | ||
public static StreamData Format(StreamEntry entry, ILogger logger) | ||
{ | ||
logger.LogInformation("ID: {val}", entry.Id.ToString()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are we logging in a Format
method? Is this best practice?
namespace Microsoft.Azure.WebJobs.Extensions.Redis.Samples.CosmosDB.Models | ||
{ | ||
public record CosmosDBListData( | ||
string id, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These should ideally start with capital letters by convention. This applies to all the models
Dictionary<string, string> dict = entry.Values.ToDictionary(value => value.Name.ToString(), value => value.Value.ToString()); | ||
|
||
// Create a new list of messages | ||
var list = new Dictionary<string, Dictionary<string, string>>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can simplify to var list = new Dictionary<string, Dictionary<string, string>> { { entry.Id.ToString(), dict } };
list.Remove(minKey); | ||
} | ||
|
||
StreamDataSingleDocument data = new StreamDataSingleDocument { id = results.id, maxlen = results.maxlen, messages = list }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can simplify to new()
// Format the key/value pairs | ||
foreach (KeyValuePair<string, string> entry in document.values) | ||
{ | ||
values[i++] = new NameValueEntry(entry.Key, entry.Value); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There must be a better way to do this, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe create a list first then convert to array?
if (cosmosData == null || cosmosData.Count <= 0) return; | ||
|
||
IDatabaseAsync redisDb = s_redisConnection.Value.GetDatabase(); | ||
//for each item upladed to cosmos, write it to Redis |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
spelling again
|
||
resultsHolder.Add(listEntry); | ||
CosmosDBListData newEntry = new CosmosDBListData(id: ListKey, value: resultsHolder); | ||
await db.UpsertItemAsync<CosmosDBListData>(newEntry); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can remove <CosmosDBListData>
timestamp: DateTime.UtcNow | ||
); | ||
|
||
logger.LogInformation($"Key: \"{newKey}\", Value: \"{cosmosDBOut.value}\" addedd to Cosmos DB container: \"{Environment.GetEnvironmentVariable(ContainerSetting.Replace("%",""))}\" at id: \"{cosmosDBOut.id}\""); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
spelling
@@ -17,7 +17,7 @@ public static StreamData Format(StreamEntry entry, ILogger logger) | |||
logger.LogInformation("ID: {val}", entry.Id.ToString()); | |||
|
|||
// Map each key value pair | |||
Dictionary<string, string> dict = RedisUtilities.StreamEntryToDictionary(entry); | |||
Dictionary<string, string> dict = entry.Values.ToDictionary(value => value.Name.ToString(), value => value.Value.ToString()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are we not using this method anymore?
Updating the samples to be more in line with the rest of the samples.