Skip to content

Conversation

@kentis
Copy link

@kentis kentis commented Jan 4, 2024

Thanks for contribution! Please go through following checklist before sending PR.

This PR fixes issue #1215

PR Branch Destination

  • For Azurite V3, please send PR to main branch.
  • For legacy Azurite V2, please send PR to legacy-dev branch.

Always Add Test Cases

Test case is added but skipped since it requires modifications to the azure table client dependency to run properly. This is because the client perform the same check. This is, howevernot true for all clients.

Add Change Log

Add change log for the code change in Upcoming Release section in ChangeLog.md.

Development Guideline

Please go to CONTRIBUTION.md for steps about setting up development environment and recommended Visual Studio Code extensions.

@edwin-huber
Copy link
Collaborator

Reviewing

@edwin-huber edwin-huber self-requested a review January 8, 2024 07:54
@edwin-huber edwin-huber added the table-storage Relating to Azurite table storage implementation label Jan 8, 2024
@blueww
Copy link
Member

blueww commented Jan 18, 2024

@edwin-huber

Feel free to let me know if you need any assistance on this PR reviewing.

batchContextClone: any
) {
if (this.partitionKeyMap.size > 0) {
console.log("checking for differing partition keys");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please remove console.log calls

});

//Skips this test because it requires modifying the Azure Data Tables client so that it does not perform a check for partition key consistency before submitting the batch.
it.skip("10. Batch API should fail when attempting to insert elements with different partitionkeys in same batch, @loki", async () => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please look at the table.entity.rest.test.ts file, there is an example of a batch request in there, which you can use to simulate this request.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am just updating the tests, which should make this easier for you

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is taking a while to convert all the REST tests to the new method and remove dependencies, hope to be done by the end of the week.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

table-storage Relating to Azurite table storage implementation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants