Misleading `MaxResults` documentation

[Gbd199]

MaxResults is backed by a u32, but construting it from u32::MAX results in a server error.

<QueryParameterName>maxresults</QueryParameterName><QueryParameterValue>4294967295</QueryParameterValue><MinimumAllowed>-2147483648</MinimumAllowed><MaximumAllowed>2147483647</MaximumAllowed>

Seems like it should be backed by an i32 instead.

I will note that I'm pretty sure using u32::MAX worked just fine last week, so maybe this is a new enforcement on the server side.
But in any case, even if changing it to NonZeroI32 is a breaking change, the doc comment should reflect this.

[heaths]

In which crate? What client method?

Also note all versions released to date and for a while yet are prereleases i.e., 0.x releases. There are no guarantees of not making breaking changes. Quite the opposite as we collect feedback and otherwise find better usage patterns.

[Gbd199]

@heaths Ye sorry of the lack of details. MaxResults is obviously from azure_core and encountered this while performing a list_blobs operation (thru azure_storage_blobs's container_client)

[heaths]

@vincenttran-msft would Storage even want this kept? Making something called "MaxResults" doesn't make sense to use a signed integer, but why is the service returning you can have a negative number?

At any rate, it's against our guidelines to have client-side validation on anything but required URL path parameters. So we wouldn't use a constraint such as NonZeroI32, though one could argue we shouldn't use an unsigned type either but, I think in this case, it makes sense since you can't have a negative max results. It's a count. If it was as vague as "max value" (an abritrary ceiling) I agree a signed integer would make more sense.

At any rate, @Gbd199, expect breaking changes. Nothing we shipped before last week has ever been supported and was, in fact, unofficial. We'll stabalize as we approach GA and only after GA will breaking changes occur if absolutely necessary, which should be rare-to-never for azure_core and rare for service crates (sometimes service have to make breaking changes, though we try to prevent that further downstream).

[Gbd199]

I agree NonZeroI32 is not the way to go, and that you have no obligation for client-side validaiton.
And also that it's super weird the BE seems to accept negative values, especially considering I'm quite sure it didn't just a week ago.

But, I would argue that the comment

// Azure has a soft cap on 5k. There is no harm to go over but maybe
// we should inform the user that they are specifing a value outside
// the allowed range. Right now we simply ignore it.

while making it clear there's no out-of-range handling on the client side, kinda misleads with the "There is no harm to go over" part while being backed by a u32.
I would suggest adding a warning of sorts, that the BE may be intolerable to too large of a value, like anything above the i32 range.
I'm not good with wording such things so don't have a specific suggestion, but do think making some change to the current comment would be benefitial.

[heaths]

And also that it's super weird the BE seems to accept negative values, especially considering I'm quite sure it didn't just a week ago.

That response came from the service, not the SDK. @seanmcc-msft @vincenttran-msft could you address this?