Skip to content
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

Add warnings to pr and v #116

Merged

Conversation

rodrigo-mira
Copy link
Contributor

No description provided.

@rodrigo-mira rodrigo-mira linked an issue Mar 14, 2023 that may be closed by this pull request
@rodrigo-mira rodrigo-mira requested review from CamiloBorba09 and mcamardon and removed request for CamiloBorba09 March 14, 2023 16:00
@rodrigo-mira rodrigo-mira force-pushed the 103-m15-add-warning-for-keys-that-should-not-be-sent branch from 6f6d568 to cad6d1f Compare March 15, 2023 13:35
@rodrigo-mira rodrigo-mira force-pushed the 103-m15-add-warning-for-keys-that-should-not-be-sent branch 2 times, most recently from 5548216 to b1a4dab Compare March 15, 2023 14:32
@rodrigo-mira rodrigo-mira force-pushed the 103-m15-add-warning-for-keys-that-should-not-be-sent branch from b1a4dab to fd3bd9a Compare March 16, 2023 13:07
@CamiloBorba09 CamiloBorba09 self-requested a review March 16, 2023 20:29
Comment on lines 378 to 385
warnings: [
{
type: 'pr-value',
key: 'pr',
value: 1,
description: 'SHOULD only be sent if not equal to 1.'
}
],
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do we have this warning? The input is

https://dash.akamaized.net/akamai/bbb_30fps/bbb_a64k/bbb_a64k_7.m4a?CMCD=bl%3D21300%2Cbr%3D3200%2Cbs%2Ccid%3D%22faec5fc2-ac30-11ea-bb37-0242ac130002%22%2Cd%3D4004%2Cdl%3D18500%2Cmtp%3D48100%2Cnor%3D%22%252F300kbps%252Ftrack.m4v%22%2Cnrr%3D%2212323-48763%22%2Cot%3Dv%2Cpr%3D1.08%2Crtp%3D12000%2Csf%3Dd%2Csid%3D%226e2fb550-c457-11e9-bb97-0800200c9a66%22%2Cst%3Dv%2Csu%2Ctb%3D6000

If you check the query pr=1.08

Comment on lines 415 to 422
warnings: [
{
type: 'pr-value',
key: 'pr',
value: 1,
description: 'SHOULD only be sent if not equal to 1.'
}
],
Copy link
Collaborator

Choose a reason for hiding this comment

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

Here is the same as above. pr = 1.08

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think there's something wrong here. In the queries pr=1.08 and we have the warning SHOULD only be sent if not equal to 1.

Another thing to consider is the there's something wrong in the parser because if you see pr=1

Copy link
Collaborator

Choose a reason for hiding this comment

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

This ticket #119 has been created to solve this bug

@@ -70,12 +70,16 @@ export const cmcdHeader = {

export const warningTypes = {
noAlphabeticalOrder: 'no-alphabetical-order',
valuePR: 'pr-value',
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would change valuePR to valuePr

noSidReceived: 'no-sid-received',
blWithWrongOtValue: 'bl-with-wrong-ot-value',
};

export const warningDescription = {
noAlphabeticalOrder: 'Keys are not arranged alphabetically',
valuePR: 'SHOULD only be sent if not equal to 1.',
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would change SHOULD only be sent if not equal to 1 to pr should only be sent if not equal to 1

noSidReceived: 'no-sid-received',
blWithWrongOtValue: 'bl-with-wrong-ot-value',
};

export const warningDescription = {
noAlphabeticalOrder: 'Keys are not arranged alphabetically',
valuePR: 'SHOULD only be sent if not equal to 1.',
valueV: 'Client SHOULD omit this field if the version is 1',
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would change 'Client SHOULD omit this field if the version is 1' to 'Client should omit this field if the version is 1'

@CamiloBorba09 CamiloBorba09 merged commit 07651cf into develop Mar 16, 2023
@CamiloBorba09 CamiloBorba09 deleted the 103-m15-add-warning-for-keys-that-should-not-be-sent branch March 16, 2023 20:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

M1.5: Add warning for keys that should not be sent
4 participants