CS3099JH2017 / cs3099jh

CS3099 Junior Honours Project Protocol and Discussion Central Repo
1 stars 6 forks source link

Proposed spec for backend (DRAFT) #19

Closed rtw43 closed 6 years ago

rtw43 commented 6 years ago

a big ugly markdown file that really needs: A) spellchecking B) splitting into smaller ones (for stuff in forewords esp) C) an author with more motivation to do the above ;)

but outlines my idea of backend specification

shiyah commented 6 years ago

Small thing: Project grants do not issue an error if the new access level does not exist

rtw43 commented 6 years ago

@shiyah The intention here was that this would be considered an invalid request (and be dealt with as specified in the basic response section). I have clarified the text in my local copy and will release an updated version soon, under a new version number.

This new version will also provide an endpoint for the client to list all valid access_levels and privileges.

Does this seem appropriate? Would you prefer a customer error along the lines of "invalid_access_level" or "access_level_not_found"?

shiyah commented 6 years ago

@rtw43 In theory, the front-end would query the API for access_levels beforehand and only allow those to be chosen, making an invalid access_level likely only possible in two scenarios:

In both cases, an invalid_request response would not be as clear as to what the error is (did the request not match the pattern or is the access_level wrong?). Splitting access_level_not_found off therefore makes a good amount of sense.

Also, thank you for the sheer amount of work you put into making the backend spec.

rtw43 commented 6 years ago

@shiyah This reasoning is fair enough, as part of the 0.1.3 release I have added a specific error for this case (as well as the equivalent case when an invalid privilege is specified).

rtw43 commented 6 years ago

I have made some additional resources for BE01 available here https://rw86.host.cs.st-andrews.ac.uk/jh/extra_resources.md.html

db213 commented 6 years ago

Umm... out of curiosity, what was wrong with the ML protocol I had written, and why couldn't I just have fixed whatever issues there were instead of completely rewriting it? (I'm sorry, not trying to be mean and definitely not claiming what I did was perfect, but it really doesn't feel good to have done some work and just to have it ignored and rewritten...)

https://swaggerhub.com/apis/JH-Project/machine-learning-api/1.0

rtw43 commented 6 years ago

Hi,

I am sorry - I don't wish to be seen as being combative, overreaching or counter productive.

Your existing protocol notes were indeed my source of inspiration (many thanks), and my original intention was to present my work as a development of them directly. However, I found myself approaching the problem from a fundamentally different position than the original notes did, in some ways like a job centric view rather than a model centric view, a different view of the role of backend/hci, such as who executes the calls, who stores/responds with the data, what kinds of tasks ML will perform, if ML is responsible for generating the report, what format the output should take, and several other things that I note under the "Acknowledgements" section.

I don't know if these changes make the design "better" but I do know that they are different to what you intended when writing the notes - and I wish people to contrast them side by side. I really do want to hear about all the reasons these changes are stupid and why "functionality x belongs instead in component y" and why "the jobs model captures the use case x badly".

This lead me to feel that the changes would be better served if presented as a alternative architecture: I no longer felt comfortable proposing such overarching changes to your work (since it seemed to conflict with some of your design intentions when you originally wrote it). My hope is that the relative merits of both approaches will be considered, I am of the belief that the best engineering occurs when several people come up with competing approaches and architectures. And think the year as whole will benefit more from exploring several different architectures than from exploring several revisions of the same one. In particular I want to give you an opportunity to show how "we can achieve the functionality you listed here much better if we instead adapt my design like so".

I am sorry if it comes across as trying to overrule your work, my intention was quite the opposite! To recognise that these are two fundamentally different designs and encourage people to think about them and debate them as such. I do not feel it would have been appropriate to pose my work as a development of yours (and if I were in your position I would have been more offended by the attempt to pass off a fundamental rearchitecture as an "improvement").

I should note: I have been liberal in slapping the word "proposal" all over everything I write, and I am under no disillusions that the year have to or even should go with them - but I did want an opportunity to present a cohesive proposal, that reflected how things fit together in my head. I encourage you to critique as much as possible and hope that I have not caused animosity.

Best, Ryan <3

db213 commented 6 years ago

Hi Ryan, thanks for the response. :) I didn't mean to make you feel bad, it was just a bit of jolt to see this when I finally started looking at JH stuff again. Having an alternative is good, I just feel like it's hard to compare when mine was literally only the ML protocol.

I'll start reading this over and leaving comments on it. :) Edit: sorry I left loads of comments, I really hope I don't come off as harsh, I don't mean to.

db213 commented 6 years ago

This should not have been accepted without the comments being resolved...? Edit: I'm an idiot. approving != merging

CodingCellist commented 6 years ago

@rtw43 Could you respond to the comments please? :-)

Also, ML will need data from BE. How do you intend to do that when ML and BE should not communicate according to your proposal?

rtw43 commented 6 years ago

Literally in the processes of doing so as you typed that

rtw43 commented 6 years ago

@teh6

Also, ML will need data from BE. How do you intend to do that when ML and BE should not communicate according to your proposal?

I muck up of wordings on my part. My intention is that ML acts as a normal BE client (using the standard file APIs that HCI uses), what I meant was that there is no special protocol only spoken between BE and HCI.

db213 commented 6 years ago

So I'm realising that I misunderstood quite a bit of this - with changes to make it fit the structure of HCI -> BE -> ML, I'm wondering if it would actually be possible to still use (possibly an edited version of) what I came up with? I don't actually see anything too drastically different to make that impossible and I would feel a lot better to know it might still get some use/not have been wasted effort.

That being said, I didn't understand your concept of "jobs", and how they differ from the model-centric idea I used, so maybe that's where the difference lies?

rtw43 commented 6 years ago

Unfortunately, due to increasing time pressure and other priorities in my life, I will no longer be seeking to merge my proposals, and will be withdrawing my pull request imminently. While my work was originally submitted with the best of intentions, I understand it has not been wholly accepted as such. My work will still be accessible via the original repo, so it may remain as a resource for anybody who wishes to utilize it. I continue to believe that the current protocols have significant issues, both architecturally and in terms of clarity and assumptions made, which I have already devoted some weeks to attempting to convey. Whilst I am no longer able to devote such large amounts of time to this, I urge continued revision and increased documentation so that these issues may both be made evident and hopefully resolved. Shortly, I will update my proposals to give explicit permission for editing and derivative works (subject to some minor conditions). I am open to anybody building their own proposal on the basis of mine, or if they possess an opinion on the matter, resubmitting my proposal; but I do not feel I currently have the time (or the popular backing) to devote the necessary attention to defending my pull request.

Without further motive, I would like to comment on my personal disagreement with the current governance structure of the project - wherein major decisions are made in private channels and discussions with unelected members, rather than in a public forum. I think we can all agree it would be best to debate publicly in order to give everyone the opportunity to engage and input, and in the hopes of encouraging merit based discussion that will ultimately result in the best project for all of us.

With thanks for your time and engagement,

theoden8 commented 6 years ago

I do think that Ryan's spec is a good attempt to improve the existing project. I myself do not feel I have the competency to standardize or assess a viable protocol, being a Joint Honours student. However, I would suggest that we move the discussion into the meeting rather than have an argument amongst the few people who are a very limited representation of students on the module, and therefore could be prone to bias towards personal credit.

I therefore ask to hold the withdrawal until we have a meeting, which, hopefully, will be organized shortly.

Best, Kirill

magnostherobot commented 6 years ago

Consideration of this protocol documentation should indeed be part of tomorrow's meeting's discussion. Even with the pull request withdrawn, the forked repository can itself be forked, and this fork edited (if necessary) and merged with the central repository.

On 29 Jan 2018 18:15, "Kirill Rodriguez" notifications@github.com wrote:

Hello,

I do think that Ryan's spec is a good attempt to improve the existing project. I myself do not feel I have the competency to standardize or assess a viable protocol, being a Joint Honours student. However, I would suggest that we move the discussion into the meeting rather than have an argument amongst the few people who are a very limited representation of students on the module, and therefore is prone to bias towards personal credit.

I therefore ask to hold the withdrawal until we have a meeting, which, hopefully, will be organized shortly.

Best, Kirill

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/CS3099JH2017/cs3099jh/pull/19#issuecomment-361335993, or mute the thread https://github.com/notifications/unsubscribe-auth/AXkuhbVz2hO7JMrbB5rCmCpR9syL7M9Aks5tPgrMgaJpZM4RQ5eR .

CodingCellist commented 6 years ago

@rtw43 Dear Ryan,

Firstly, we would like to apologise if Thomas’ e-mail caused you any offense. There seems to have been a misunderstanding in terms of what we wanted to achieve by asking you to stick to the agreed protocols which we believed had been agreed through an open discussion involving all project participants. Below we have tried to outline how to move forward by merging the work done by you with the work done by others and the structure jointly agreed by all participants.

Our starting point has always been that it is best to discuss things publicly. This is why we initially held a StrawPoll on the preferred format for such discussions, the conclusion of which was that a number of meetings open to all should be held. Three such meetings took place throughout the first semester, and the current structure was agreed by the approx. 20 participants at the final one: Namely that HCI would communicate only with BE and that ML would communicate only with BE (i.e. the HCI <=> BE <=> ML structure). Any reservations with this approach could have been raised either in that meeting or following the subsequent distribution of the minutes.

However, the considerable work you have done on the protocols is not incompatible with the approach above, and we by no means want to discard what you have done. On the contrary, we think it could be useful to merge it with the existing work done by Daphne as obviously otherwise there will be no APIs or protocols for HCI or BE. However, the overall structure must remain the one that was decided by all in the final meeting before the holidays.

If there are issues with the current protocols – and we share your point that there is – we do indeed need to address them. However, it is important that such issues are raised on Git, i.e. on the official repo and not a fork, since Git allows people to know that there is an issue, to discuss alternatives, to put themselves down as developers, to easily track what is still open and what has been resolved and by whom, etc.

We hope that the above is acceptable to you and look forward to continue working together on this complex project.

All the best, Thomas Hansen, Daphne Bogosian, and Aidan McGinley

Knepsia commented 6 years ago

As I see it, there was no protocol here for Back End or Front End, and Ryan provided one for those two, he also further defined the existing Machine Learning protocol, explaining it more clearly and making the changes as was agreed in the meetings. While I agree it should all be kept in the repository where all the protocols are created, Ryan did so, he made a pull request 28 days ago... So what was the issue here?

magnostherobot commented 6 years ago

I'm posting this for the sake of clarity:

This has all now been discussed in a meeting held today. Both sets of documentation are to be merged, and discrepancies highlighted as issues and addressed.