[Engineering] Update Dependency Versions

[jsquire]

Summary

The intent of these changes is to set the foundation for dependencies used by the forthcoming Event Hubs Blob Checkpoint Store library.

Goals

• Add package versions for Event Hubs and Blob Storage to be used by the Event Hubs Blob Checkpoint Store projects.

• Updating Storage Management version to allow for interacting with blob containers.

Last Upstream Rebase

Wednesday, September 11, 2019 9:03am (EDT)

Related and Follow-Up Issues

• [Epic] Release Event Hubs Track 2 Library Preview 3 for .Net (Release Event Hubs Track 2 Library Preview 3 for .Net #7141)

[jsquire]

According to the .NET Dependency Report there are no current consumers of the Microsoft.Azure.Management.Storage library, so I believe it should be save to rev the version.

[jsquire]

The intent is to wait to merge these changes until tomorrow, once Preview 3 of the Azure Storage client library has been confirmed as released.

[tg-msft]

We're scheduled to publish Preview 3 this afternoon so it might be good to hold off on merging until then.

[weshaggard]

The intent of these changes is to set the foundation for dependencies used by the forthcoming Event Hubs Blob Checkpoint Store library.

Why do it preemptively? I think it would be nice to go it with the actual changes that consume these.

[jsquire]

The intent of these changes is to set the foundation for dependencies used by the forthcoming Event Hubs Blob Checkpoint Store library.

Why do it preemptively? I think it would be nice to go it with the actual changes that consume these.

I have a follow-up PR that uses these, waiting only for this to clear, so it's not long-term planning; there's an immediate need. Why do it separately? To try and keep concerns separated and smoke out any conflicts - like the failures that submitting this exposed. That lets me potentially address the conflict without introducing it to the main set of changes to which it would be orthogonal.

[weshaggard]

To try and keep concerns separated and smoke out any conflicts - like the failures that submitting this exposed. That lets me potentially address the conflict without introducing it to the main set of changes to which it would be orthogonal.

What failures were exposed by submitting this as is without any consumers?

My main thing is that I don't want these dependencies listed in here without having consumers as they will only cause folks pause later if someone needs to update one, similar to the one you updated that didn't have any consumers any longer. We really need a process to GC the unused versions in this file.

By adding these dependencies at the same time as you add the consuming code it makes it clear why they are being added so I still suggest doing them together.

[jsquire]

To try and keep concerns separated and smoke out any conflicts - like the failures that submitting this exposed. That lets me potentially address the conflict without introducing it to the main set of changes to which it would be orthogonal.

What failures were exposed by submitting this as is without any consumers?

My main thing is that I don't want these dependencies listed in here without having consumers as they will only cause folks pause later if someone needs to update one, similar to the one you updated that didn't have any consumers any longer. We really need a process to GC the unused versions in this file.

By adding these dependencies at the same time as you add the consuming code it makes it clear why they are being added so I still suggest doing them together.

Sorry. This didn't come up until the merge refresh; wasn't my intent to work around the discussion.

The failures were due to expectations in the HD Insight recorded tests which appear to have been last generated in late 2018. The API remained the same, but the version parameter needed to be updated.

I understand the concern and acknowledge the validity, but at the same time, there is also value in keeping PRs to related work and keeping the size from ballooning. The set of changes that this was gating is not small, and has some time considerations around the associated release; my goal was to keep that as streamlined and accessible for review as possible.

[weshaggard]

Sounds reasonable at least for the one dependency update, I must have missed the test recording update when I reviewed the change.

I do still stand by general guidance that we should try to keep the version changes and new changes to this file with the new consumers of the changes so it is clear why we are changing them. The first question I will have anytime I see changes to this versions file is why it is needed and having the associated changes with it make that clearer.

[jsquire]

Sounds reasonable at least for the one dependency update, I must have missed the test recording update when I reviewed the change.

I do still stand by general guidance that we should try to keep the version changes and new changes to this file with the new consumers of the changes so it is clear why we are changing them. The first question I will have anytime I see changes to this versions file is why it is needed and having the associated changes with it make that clearer.

Understood and no disagreement in the general case. I'll continue to use that as the desired state unless there's a justification to deviate.