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

Deprecate school names api method #18

Open
villarin opened this issue Jul 9, 2013 · 7 comments
Open

Deprecate school names api method #18

villarin opened this issue Jul 9, 2013 · 7 comments
Assignees

Comments

@villarin
Copy link
Collaborator

villarin commented Jul 9, 2013

Api method: http://pythondave.github.io/th-admin/2/0.4/scaffolding/api/default.html#/school-names

Please deprecate this method and include the school names data as a new property of: /admin/service/basic-lists.

School names records can be kept pretty much as they are now - but would be good to add the id attribute as well.

See proposed format for the school names property below:

"schoolNames": [
{
"id": 1,
"name": "School of Manuel"
},
{
"id": 2,
"name": "Ecole de Marissa"
}]

@ghost ghost assigned pythondave Jul 9, 2013
@pythondave
Copy link
Owner

I'm keen to keep discussions about the API more discussional than
directive. I hope you see the value in this approach.

My first goal of any discussion is to try to reach a common understanding.
I'm not sure we have that yet, so this post is focussed only on the
achievement of that goal - and I'll start things off with an example.

Let's say (for sake of this example), that we have data on the server that
looks like this:

schools
id name
1 School A
2 School A
3 School A
4 School A
5 School A
6 School A
7 School A
8 School B
9 School B
10 School B

With this data, as I understand things, you're proposing an API method
which returns something like this ($):

"schoolNames": [
{ id: "1", name: "School A" },
{ id: "2", name: "School A" },
{ id: "3", name: "School A" },
{ id: "4", name: "School A" },
{ id: "5", name: "School A" },
{ id: "6", name: "School A" },
{ id: "7", name: "School A" },
{ id: "8", name: "School B" },
{ id: "9", name: "School B" },
{ id: "10", name: "School B" }]

I can do everything I need to on this sprint with this (if I refactor), and
therefore this would be entirely sufficient.

Note, however, that the only information needed on this sprint from the
example server data is: "School A", "School B"

Before I go on, I think it's worthwhile pausing here and getting
confirmation that I've haven't incorrectly represented what you're trying
to get across, and that you understand what I've tried to get across so
far. I'll continue once it's clear we have a firm base.

Please confirm! :)

On 9 July 2013 11:07, villarin notifications@github.com wrote:

Api method:
http://pythondave.github.io/th-admin/2/0.4/scaffolding/api/default.html#/school-names

Please deprecate this method and include the school names data as a new
property of: /admin/service/basic-lists.

School names records can be kept pretty much as they are now - but would
be good to add the id attribute as well.

See proposed format for the school names property below:

"schoolNames": [
{
"id": 1,
"name": "School of Manuel"
},
{
"id": 2,
"name": "Ecole de Marissa"
}]


Reply to this email directly or view it on GitHubhttps://github.com//issues/18
.

@villarin
Copy link
Collaborator Author

villarin commented Jul 9, 2013

I think that the only important bit for this sprint would be to keep the
school names as part of the basic lists - to ensure they enjoy the same
treatment as other basic lists i.e. aggressive caching and compression for
faster access.

Adding the "ID" to schools - is not really required at this stage - but I
thought it would be nice to have as it is likely we will use it for server
side filtering purposes in some of the upcoming sprints.


From: Ryan Randall [mailto:notifications@github.com]
Sent: 09 July 2013 13:35
To: pythondave/th-admin
Cc: villarin
Subject: Re: [th-admin] Deprecate school names api method (#18)

I'm keen to keep discussions about the API more discussional than
directive. I hope you see the value in this approach.

My first goal of any discussion is to try to reach a common understanding.
I'm not sure we have that yet, so this post is focussed only on the
achievement of that goal - and I'll start things off with an example.

Let's say (for sake of this example), that we have data on the server that
looks like this:

schools
id name
1 School A
2 School A
3 School A
4 School A
5 School A
6 School A
7 School A
8 School B
9 School B
10 School B

With this data, as I understand things, you're proposing an API method
which returns something like this ($):

"schoolNames": [
{ id: "1", name: "School A" },
{ id: "2", name: "School A" },
{ id: "3", name: "School A" },
{ id: "4", name: "School A" },
{ id: "5", name: "School A" },
{ id: "6", name: "School A" },
{ id: "7", name: "School A" },
{ id: "8", name: "School B" },
{ id: "9", name: "School B" },
{ id: "10", name: "School B" }]

I can do everything I need to on this sprint with this (if I refactor), and
therefore this would be entirely sufficient.

Note, however, that the only information needed on this sprint from the
example server data is: "School A", "School B"

Before I go on, I think it's worthwhile pausing here and getting
confirmation that I've haven't incorrectly represented what you're trying
to get across, and that you understand what I've tried to get across so
far. I'll continue once it's clear we have a firm base.

Please confirm! :)

On 9 July 2013 11:07, villarin notifications@github.com wrote:

Api method:

http://pythondave.github.io/th-admin/2/0.4/scaffolding/api/default.html#/sch
ool-names

Please deprecate this method and include the school names data as a new
property of: /admin/service/basic-lists.

School names records can be kept pretty much as they are now - but would
be good to add the id attribute as well.

See proposed format for the school names property below:

"schoolNames": [
{
"id": 1,
"name": "School of Manuel"
},
{
"id": 2,
"name": "Ecole de Marissa"
}]

Reply to this email directly or view it on
GitHubhttps://github.com//issues/18
.

Reply to this email directly or view
#18 (comment) it
on GitHub.
<https://github.com/notifications/beacon/C6aQ2lOetMBBdndp2ctui8-WRdVfNu7T0Ui
MwcZI5XaHz32JPMbJ7iAfx2WSr2ex.gif>

@pythondave
Copy link
Owner

It's still not clear to me from your reply that you understand what I'm
trying to get across.

Can you please confirm that you understand that the difference between $
and $$ is not simply the existence or not of the id attribute. Note that $
contains 10 items whereas $$ contains 2 items.

On 9 July 2013 14:51, villarin notifications@github.com wrote:

I think that the only important bit for this sprint would be to keep the
school names as part of the basic lists - to ensure they enjoy the same
treatment as other basic lists i.e. aggressive caching and compression for
faster access.

Adding the "ID" to schools - is not really required at this stage - but I
thought it would be nice to have as it is likely we will use it for server
side filtering purposes in some of the upcoming sprints.


From: Ryan Randall [mailto:notifications@github.com]
Sent: 09 July 2013 13:35
To: pythondave/th-admin
Cc: villarin
Subject: Re: [th-admin] Deprecate school names api method (#18)

I'm keen to keep discussions about the API more discussional than
directive. I hope you see the value in this approach.

My first goal of any discussion is to try to reach a common understanding.
I'm not sure we have that yet, so this post is focussed only on the
achievement of that goal - and I'll start things off with an example.

Let's say (for sake of this example), that we have data on the server that
looks like this:

schools
id name
1 School A
2 School A
3 School A
4 School A
5 School A
6 School A
7 School A
8 School B
9 School B
10 School B

With this data, as I understand things, you're proposing an API method
which returns something like this ($):

"schoolNames": [
{ id: "1", name: "School A" },
{ id: "2", name: "School A" },
{ id: "3", name: "School A" },
{ id: "4", name: "School A" },
{ id: "5", name: "School A" },
{ id: "6", name: "School A" },
{ id: "7", name: "School A" },
{ id: "8", name: "School B" },
{ id: "9", name: "School B" },
{ id: "10", name: "School B" }]

I can do everything I need to on this sprint with this (if I refactor),
and
therefore this would be entirely sufficient.

Note, however, that the only information needed on this sprint from the
example server data is: "School A", "School B"

Before I go on, I think it's worthwhile pausing here and getting
confirmation that I've haven't incorrectly represented what you're trying
to get across, and that you understand what I've tried to get across so
far. I'll continue once it's clear we have a firm base.

Please confirm! :)

On 9 July 2013 11:07, villarin notifications@github.com wrote:

Api method:

http://pythondave.github.io/th-admin/2/0.4/scaffolding/api/default.html#/sch
ool-names

Please deprecate this method and include the school names data as a new
property of: /admin/service/basic-lists.

School names records can be kept pretty much as they are now - but would
be good to add the id attribute as well.

See proposed format for the school names property below:

"schoolNames": [
{
"id": 1,
"name": "School of Manuel"
},
{
"id": 2,
"name": "Ecole de Marissa"
}]

Reply to this email directly or view it on
GitHubhttps://github.com//issues/18
.

Reply to this email directly or view
#18 (comment)
it
on GitHub.
<
https://github.com/notifications/beacon/C6aQ2lOetMBBdndp2ctui8-WRdVfNu7T0Ui
MwcZI5XaHz32JPMbJ7iAfx2WSr2ex.gif>


Reply to this email directly or view it on GitHubhttps://github.com//issues/18#issuecomment-20674993
.

@villarin
Copy link
Collaborator Author

villarin commented Jul 9, 2013

Yes I understand that - but this is why id is a unique identifier for each
school - but that is only important for filtering purposes.

As far as the names are concerned a list of unique names can be derived from
the main school list on the client side - specially using the data handling
dependencies - of the project i.e. lodash

Or maybe the school name should just have the location appended to it - so
the ambiguities can be eliminated...


From: Ryan Randall [mailto:notifications@github.com]
Sent: 09 July 2013 15:06
To: pythondave/th-admin
Cc: villarin
Subject: Re: [th-admin] Deprecate school names api method (#18)

It's still not clear to me from your reply that you understand what I'm
trying to get across.

Can you please confirm that you understand that the difference between $
and $$ is not simply the existence or not of the id attribute. Note that $
contains 10 items whereas $$ contains 2 items.

On 9 July 2013 14:51, villarin notifications@github.com wrote:

I think that the only important bit for this sprint would be to keep the
school names as part of the basic lists - to ensure they enjoy the same
treatment as other basic lists i.e. aggressive caching and compression for

faster access.

Adding the "ID" to schools - is not really required at this stage - but I
thought it would be nice to have as it is likely we will use it for server

side filtering purposes in some of the upcoming sprints.


From: Ryan Randall [mailto:notifications@github.com]
Sent: 09 July 2013 13:35
To: pythondave/th-admin
Cc: villarin
Subject: Re: [th-admin] Deprecate school names api method (#18)

I'm keen to keep discussions about the API more discussional than
directive. I hope you see the value in this approach.

My first goal of any discussion is to try to reach a common understanding.

I'm not sure we have that yet, so this post is focussed only on the
achievement of that goal - and I'll start things off with an example.

Let's say (for sake of this example), that we have data on the server that

looks like this:

schools
id name
1 School A
2 School A
3 School A
4 School A
5 School A
6 School A
7 School A
8 School B
9 School B
10 School B

With this data, as I understand things, you're proposing an API method
which returns something like this ($):

"schoolNames": [
{ id: "1", name: "School A" },
{ id: "2", name: "School A" },
{ id: "3", name: "School A" },
{ id: "4", name: "School A" },
{ id: "5", name: "School A" },
{ id: "6", name: "School A" },
{ id: "7", name: "School A" },
{ id: "8", name: "School B" },
{ id: "9", name: "School B" },
{ id: "10", name: "School B" }]

I can do everything I need to on this sprint with this (if I refactor),
and
therefore this would be entirely sufficient.

Note, however, that the only information needed on this sprint from the
example server data is: "School A", "School B"

Before I go on, I think it's worthwhile pausing here and getting
confirmation that I've haven't incorrectly represented what you're trying
to get across, and that you understand what I've tried to get across so
far. I'll continue once it's clear we have a firm base.

Please confirm! :)

On 9 July 2013 11:07, villarin notifications@github.com wrote:

Api method:

http://pythondave.github.io/th-admin/2/0.4/scaffolding/api/default.html#/sch

ool-names

Please deprecate this method and include the school names data as a new
property of: /admin/service/basic-lists.

School names records can be kept pretty much as they are now - but would

be good to add the id attribute as well.

See proposed format for the school names property below:

"schoolNames": [
{
"id": 1,
"name": "School of Manuel"
},
{
"id": 2,
"name": "Ecole de Marissa"
}]

Reply to this email directly or view it on
GitHubhttps://github.com//issues/18
.

https://github.com/notifications/beacon/C6aQ2lOetMBBdndp2ctui8-WRdVfNu7T0Ui
MwcZI5XaHz32JPMbJ7iAfx2WSr2ex.gif>

Reply to this email directly or view it on
GitHub<#18 (comment)
3>
.

Reply to this email directly or view
#18 (comment) it
on GitHub.
<https://github.com/notifications/beacon/C6aQ2lOetMBBdndp2ctui8-WRdVfNu7T0Ui
MwcZI5XaHz32JPMbJ7iAfx2WSr2ex.gif>

@pythondave
Copy link
Owner

Okay - it seems we have that firm base I was striving for. I'll now
continue.

You've confirmed that I understand your proposal.

I'm definitely not averse to having an API method exactly as you describe.

My concern is that if I use it for this sprint then it goes against another
of my API design approach rules - namely Rule C) Get only what you need.

With this proposal we're getting $ when we need only $$.

Can you confirm that my understanding is correct - i.e. that this proposal
does break Rule C.

If this is the case, then we have a similar situation to issue #17.

How about you have a go at proposing a replacement rule / guideline / set
of guidelines for Rule C?

On 9 July 2013 15:17, villarin notifications@github.com wrote:

Yes I understand that - but this is why id is a unique identifier for each
school - but that is only important for filtering purposes.

As far as the names are concerned a list of unique names can be derived
from
the main school list on the client side - specially using the data handling
dependencies - of the project i.e. lodash

Or maybe the school name should just have the location appended to it - so
the ambiguities can be eliminated...


From: Ryan Randall [mailto:notifications@github.com]
Sent: 09 July 2013 15:06

To: pythondave/th-admin
Cc: villarin
Subject: Re: [th-admin] Deprecate school names api method (#18)

It's still not clear to me from your reply that you understand what I'm
trying to get across.

Can you please confirm that you understand that the difference between $
and $$ is not simply the existence or not of the id attribute. Note that $
contains 10 items whereas $$ contains 2 items.

On 9 July 2013 14:51, villarin notifications@github.com wrote:

I think that the only important bit for this sprint would be to keep the
school names as part of the basic lists - to ensure they enjoy the same
treatment as other basic lists i.e. aggressive caching and compression
for

faster access.

Adding the "ID" to schools - is not really required at this stage - but
I
thought it would be nice to have as it is likely we will use it for
server

side filtering purposes in some of the upcoming sprints.


From: Ryan Randall [mailto:notifications@github.com]
Sent: 09 July 2013 13:35
To: pythondave/th-admin
Cc: villarin
Subject: Re: [th-admin] Deprecate school names api method (#18)

I'm keen to keep discussions about the API more discussional than
directive. I hope you see the value in this approach.

My first goal of any discussion is to try to reach a common
understanding.

I'm not sure we have that yet, so this post is focussed only on the
achievement of that goal - and I'll start things off with an example.

Let's say (for sake of this example), that we have data on the server
that

looks like this:

schools
id name
1 School A
2 School A
3 School A
4 School A
5 School A
6 School A
7 School A
8 School B
9 School B
10 School B

With this data, as I understand things, you're proposing an API method
which returns something like this ($):

"schoolNames": [
{ id: "1", name: "School A" },
{ id: "2", name: "School A" },
{ id: "3", name: "School A" },
{ id: "4", name: "School A" },
{ id: "5", name: "School A" },
{ id: "6", name: "School A" },
{ id: "7", name: "School A" },
{ id: "8", name: "School B" },
{ id: "9", name: "School B" },
{ id: "10", name: "School B" }]

I can do everything I need to on this sprint with this (if I refactor),
and
therefore this would be entirely sufficient.

Note, however, that the only information needed on this sprint from the
example server data is: "School A", "School B"

Before I go on, I think it's worthwhile pausing here and getting
confirmation that I've haven't incorrectly represented what you're
trying
to get across, and that you understand what I've tried to get across so
far. I'll continue once it's clear we have a firm base.

Please confirm! :)

On 9 July 2013 11:07, villarin notifications@github.com wrote:

Api method:

http://pythondave.github.io/th-admin/2/0.4/scaffolding/api/default.html#/sch

ool-names

Please deprecate this method and include the school names data as a
new
property of: /admin/service/basic-lists.

School names records can be kept pretty much as they are now - but
would

be good to add the id attribute as well.

See proposed format for the school names property below:

"schoolNames": [
{
"id": 1,
"name": "School of Manuel"
},
{
"id": 2,
"name": "Ecole de Marissa"
}]

Reply to this email directly or view it on
GitHubhttps://github.com//issues/18
.

Reply to this email directly or view
#18 (comment)

it
on GitHub.
<

https://github.com/notifications/beacon/C6aQ2lOetMBBdndp2ctui8-WRdVfNu7T0Ui
MwcZI5XaHz32JPMbJ7iAfx2WSr2ex.gif>

Reply to this email directly or view it on
GitHub<
#18 (comment)
3>
.

Reply to this email directly or view
#18 (comment)
it

on GitHub.
<
https://github.com/notifications/beacon/C6aQ2lOetMBBdndp2ctui8-WRdVfNu7T0Ui
MwcZI5XaHz32JPMbJ7iAfx2WSr2ex.gif>

Reply to this email directly or view it on GitHubhttps://github.com//issues/18#issuecomment-20676744
.

@villarin
Copy link
Collaborator Author

villarin commented Jul 9, 2013

Ok I can see that changing the format of the schoolNames schema at this
point is not really a plus for us now - in fact it may just cause more
problems...

So I think we should probably stick to Rule C on this one - and only change
the location where the schoolNames data resides or literally migrate it from
/scaffolding/api/default.html#/school-names to
/scaffolding/api/default.html#/basic-lists.

Issue #17 is different because it causes code duplication and other
organisational issues in the codebase.

@pythondave
Copy link
Owner

Understood.

I've just created #20 as a reminder to document whatever general API design rules / guidelines we come to an agreement on.

I think this is a great thing to come out of this discussion (and the one on issue 17).

I think I'm happy on this exact issue as long as we stick to Rule C - so please go with whatever works best for you.

Let me know what you decide and I'll refactor things at my end accordingly.

@ghost ghost assigned villarin Jul 11, 2013
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

2 participants