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

SI: Adding test and recovery samples and changing vac samples to one entry #281

Merged
merged 23 commits into from
Jun 8, 2021

Conversation

jozemlakar
Copy link
Contributor

No description provided.

Jože Mlakar and others added 18 commits May 12, 2021 21:55
…es#77)

This commit updates samples from Slovenia. Much has changed:
- The samples are now signed using DSC from ACC environment.
- At the moment DSC is not yet available in ACC as we are waiting for the EC team to complete the inclusion of our NB certs
- DSC is created as recommended on onboarding documents (EC)
- DGCs are created usign dgc-java project that Slovenia is using for our implementation
@jozemlakar
Copy link
Contributor Author

Hm, weird errors. As if it was not possible to get files from GitHub.

Copy link
Collaborator

@SchulzeStTSI SchulzeStTSI left a comment

Choose a reason for hiding this comment

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

@jozemlakar In file 5 is the prefix not able to decode here: https://github.pathcheck.org/debug.html can you check that?

@daniel-eder
Copy link
Member

@jozemlakar due to a change in how the EHNs JSON schema URL works, the check for your PR currently fails. Could you update your branch to the latest main to incorporate the updated and working schema URL (specifically, the test script with said updated url)? Then we can see if there are any actual issues left.

Thanks!

@jozemlakar
Copy link
Contributor Author

@daniel-eder we have merged the main and that fixed the URL error. However now we have a problem with sc field (too large of precision in sc field). Will have to wait on Monday.

@jozemlakar jozemlakar changed the title Adding test and recovery samples and changing vac samples to one entry SI: Adding test and recovery samples and changing vac samples to one entry Jun 7, 2021
@jozemlakar
Copy link
Contributor Author

Failure is due to diggsweden/dgc-java#66

@martin-lindstrom
Copy link
Collaborator

@daniel-eder @SchulzeStTSI Why is a fine grained time a problem? It is unnecessary, yes, but still. By washing away these odd cases from the test files you actually give validators less data to parse. In my meaning, this shouldn't be an error.

@jozemlakar
Copy link
Contributor Author

@martin-lindstrom while I would agree, there is also the issue of validation apps that are failing on this. So if we say, we allow fine grained time, then we are also braking verifier apps. One of those "we are right, but at what price" things.

@martin-lindstrom
Copy link
Collaborator

@martin-lindstrom while I would agree, there is also the issue of validation apps that are failing on this. So if we say, we allow fine grained time, then we are also braking verifier apps. One of those "we are right, but at what price" things.

One could argue that those validator apps should be interested in fixing the problem on their side instead of attempting to get all issuers to limit what is created...

@martin-lindstrom
Copy link
Collaborator

@daniel-eder @SchulzeStTSI About date-time...

From the specification:

The date and time when the test sample was collected. The time MUST include information on the time zone. The value MUST NOT denote the time when the test result was produced.

Exactly 1 (one) non-empty field MUST be provided.

One of the following ISO 8601 formats MUST be used. Other options are not supported.

YYYY-MM-DDThh:mm:ssZ 
YYYY-MM-DDThh:mm:ss[+-]hh 
YYYY-MM-DDThh:mm:ss[+-]hhmm 
YYYY-MM-DDThh:mm:ss[+-]hh:mm 

Examples:

"sc": "2021-08-20T10:03:12Z"	(UTC time) 
"sc": "2021-08-20T12:03:12+02"	(CEST time) 
"sc": "2021-08-20T12:03:12+0200"	(CEST time) 
"sc": "2021-08-20T12:03:12+02:00"	(CEST time) 

I heard from @jozemlakar that you expect fractions and don't accept the above? Is that correct? If so, the test scripts need to be updated.

@jozemlakar
Copy link
Contributor Author

@daniel-eder @SchulzeStTSI @martin-lindstrom this is base on failed checks when I submitted samples without fractions. Checks failed because expected value was
'sc', '2021-06-07T16:51:35.000000Z'
our value was
'sc', '2021-06-07T16:51:35Z'

@SchulzeStTSI SchulzeStTSI merged commit 522a61f into eu-digital-green-certificates:main Jun 8, 2021
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.

None yet

4 participants