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

Beta Masaheft Request Review #129

Open
PietroLiuzzo opened this issue Sep 5, 2018 · 16 comments
Open

Beta Masaheft Request Review #129

PietroLiuzzo opened this issue Sep 5, 2018 · 16 comments

Comments

@PietroLiuzzo
Copy link
Contributor

I would be very glad if somebody could have a look at my draft implementation and review it.

http://betamasaheft.eu/api/dts

@PonteIneptique
Copy link
Member

I'll have a look tomorrow or the day after at worst, as tomorrow is mostly plane.

@PonteIneptique
Copy link
Member

I don't know how complete this is supposed to be so, take this with a grain of salt :

  1. I have a broad issue with the use of urn:dts because we did not specify such a URN namespace. I think this shows that Naming authorities and Identity URNs #32 is still relevant for people looking to identify their text with a simple, stable identifier.
  2. I have not seen any issues with the Collection endpoint
    a. I would still recommend to have dc:title with @lang in situations like http://betamasaheft.eu/api/dts/collections?id=urn:dts:betmas:LIT4443HeavenlyPriests2
    b. dc:type are copied from the documentation, I don't know if this is intended or a mistake
    c. Seems like a bug : in http://betamasaheft.eu/api/dts/collections?id=urn:dts:betmasMS , dc:title is null
  3. For http://betamasaheft.eu/api/dts/document?id=urn:dts:betmas:LIT4443HeavenlyPriests2 , I have no text body
  4. I feel like if a text has no reference, it should return a 404 status : http://betamasaheft.eu/api/dts/navigation?id=urn:dts:betmas:LIT4443HeavenlyPriests2
  5. Error message are not conformant to the latest specification, though they have not been put into the documentation, here is some help Response format for missing parameters #85 ( http://betamasaheft.eu/api/dts/collections?id=urn )
  6. I don't think your root collection should be named urn:dts.
  7. I can't find a document for which navigation works : http://betamasaheft.eu/api/dts/navigation?id=urn:dts:betmasMS:DSEthiop12&level=3 . If you have not navigation for the moment, I'd recommend turning off the link to the navigation endpoint

It seems mostly like bugs, often tied to quick prototyping, nothing that goes strongly against the specs except for the error message but it's not in the Markdown yet.

I hope this helps !

@PonteIneptique
Copy link
Member

PonteIneptique commented Sep 9, 2018

Writing the notebook for demoing, I saw that http://betamasaheft.eu/api/dts/collections?id=urn:dts:betmasMS:DSEthiop1 is throwing an error. Just giving you a head's up @PietroLiuzzo :)

@PietroLiuzzo
Copy link
Contributor Author

Thanks! I will have a look at it. There might be MANY that have errors already in the XML and therefore result in problems there at some points

@PonteIneptique
Copy link
Member

@PietroLiuzzo Don't worry. Tell me when you want me to have another look at it !

@PietroLiuzzo
Copy link
Contributor Author

are 1 and 6 the same thing? I am not sure what I need to do and if this is simply wrong or what. I could use the identifier of the resource directly, but I liked the use of this common format urn:dts: etc.
on 2b, I thought that needed to be done that way. i.e. if it is an edition, I need to add that type. is that not so?
2c is a bug
3. I realize that is a faulty behaviour. I am selecting the data to be presented, selecting resources which have a div[@type='edition'] which have an ab element with some text in it as a descendant, because we do have many of this where a structure has been encoded but no text is present yet. in this case LIT4443HeavenlyPriests2 has some white space inside the ab. the request at least from my testing returns the entire document as I understood it should from the specification, where you can see this.

@PonteIneptique
Copy link
Member

1 and 6 are not exactly the same thing:

  • 1 is about the use of the dts URN namespace, which is not official but I think we should think about it.
  • 2 is about the use of a flat urn:dts: @id that would be incorrect even if the namespace existed : your root collection cannot / should not be identified by a URN namespace.

Yes, if there is no ref, start and end provided, the whole document should be returned. I was marking this because it could have been a bug in 3.

@PietroLiuzzo
Copy link
Contributor Author

I am quite sure I am missing a bit regarding the urn:dts: points.
at the moment in my implementation /collection is pointing to the root collections but if you have urn:dts: only, is redirecting to /collection . perhaps I should forward instead of redirecting? or is just part of my handling wrongly the response codes.

@PonteIneptique
Copy link
Member

We'll speak about the urn:dts issue another time, no worries. I'll try to make myself clear :)

Found another little bug : in http://betamasaheft.eu/api/dts/collections?id=urn:dts:betmasMS:BLorient718 , dc:creator is a list of list of stirng, it should be list of string :)

@PietroLiuzzo
Copy link
Contributor Author

I wish I could access my server to fix this, but it does not look like this is possible from here

@PonteIneptique
Copy link
Member

No worries @PietroLiuzzo , I am just adding not as I go on. Nothing needs to be perfect today nor this week.

Another small bug : in http://betamasaheft.eu/api/dts/navigation?id=urn:dts:betmasMS:BLorient718&ref=1&level=2 , we should only have the columns inside the folio 1.

@PonteIneptique
Copy link
Member

And http://betamasaheft.eu/api/dts/document?id=urn:dts:betmasMS:BLorient718&ref=1 does not return the abstract :)

@PietroLiuzzo
Copy link
Contributor Author

there is no abstract for the manuscript because there isn't one in the TEI. but I see it returns nothing in the fragment, which is a bug. thanks!

@PonteIneptique
Copy link
Member

my bad, French-English issue : I meant fragment :) ...

@PonteIneptique PonteIneptique changed the title request for implementation review Beta Masaheft Request Review Sep 14, 2018
@PietroLiuzzo
Copy link
Contributor Author

Thanks again @PonteIneptique I have updated my implementation with these comments.

actually, the last link was indeed broken, but the folio reference cannot be given as 1 anyway, it needs to be 1r, or 1ra if there is a column, as manuscripts are foliated, not paginated most of the time. Where should that information be to avoid that 1 is passed instead of 1r or 1v ?

@PietroLiuzzo
Copy link
Contributor Author

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

No branches or pull requests

3 participants