Skip to content

Api urls in json - #5035

Merged
jburel merged 24 commits into
ome:developfrom
will-moore:api_urls_in_json
Jan 23, 2017
Merged

Api urls in json#5035
jburel merged 24 commits into
ome:developfrom
will-moore:api_urls_in_json

Conversation

@will-moore

@will-moore will-moore commented Jan 16, 2017

Copy link
Copy Markdown
Member

What this PR does

Adds urls to the api json in the same style as github api, E.g. https://api.github.com/users/will-moore
E,g, when listing /projects/ each will have links that are absolute urls (clickable in browser)

"url:datasets": "http://localhost:4080/api/v0.1/m/projects/11601/datasets/",
"url:project": "http://localhost:4080/api/v0.1/m/projects/11601/",

Testing this PR

  1. Urls to expect:
    • /projects/ - each project has link to 'datasets' and 'project' as above.
    • /project/:id/ - has a link to 'datasets' only.
    • /datasets/ - each dataset has link to 'images and 'dataset'.
    • /dataset/ - has link to 'images' only.
    • /images/ - each image has link to 'image' only.
    • Same for Screens -> Plates (no links to Wells - not yet supported)
  2. Tests cover all urls in P->D->I in test_pdi_urls() and S>P in test_spw_urls()
    • I have ignored urls in test_api_projects.py (and now test_api_images)
    • And will have to do the same for future tests.

NB: urls can be configured to be "absolute" vv prefixed using the "omero.web.api.absolute_url" setting.

cc @chris-allan @aleksandra-tarkowska @jburel

@will-moore will-moore closed this Jan 16, 2017
@atarkowska

atarkowska commented Jan 16, 2017

Copy link
Copy Markdown
Member

regarding your question about absolute uri, my feeling was that by default we will skip domain from api, but I don't remember exactly what we said over the meeting about build_absolute_uri #4708 (comment).

EDITED: So to test urls in unittests did you try using @override_settings() see example https://github.com/openmicroscopy/openmicroscopy/pull/4706/files#diff-23c6659b076340831a8d1839cea54cfaR94. For integration tests you would need to decide global config set by bin/omero and build_absolute_uri in a test?

@atarkowska

atarkowska commented Jan 16, 2017

Copy link
Copy Markdown
Member

THAT IS AN EXAMPLE OF BUILDING ABSOLUTE URI in UNITTEST to assert values
Don't use it in django APP

To call build_absolute_uri did you try in your tests:

from django.http import HttpRequest

def request_helper(url):
  r = HttpRequest()   
  r.META = {
    "SERVER_NAME": "testserver",
    "SERVER_PORT": "80",
  }
  return r.build_absolute_uri(url)

request_helper(reverse('weblogin'))

@manics

manics commented Jan 16, 2017

Copy link
Copy Markdown
Member

Is the absolute URI based on the Host header sent by the client?

@will-moore will-moore reopened this Jan 16, 2017
@will-moore

Copy link
Copy Markdown
Member Author

@manics Are you talking about the Django test framework or 'real world'? I think it's Yes either way.

@atarkowska

atarkowska commented Jan 17, 2017

Copy link
Copy Markdown
Member

@manics yes it is. My understanding was that we will omit using build_absolute_uri anywhere unless someone configure it https://github.com/openmicroscopy/openmicroscopy/blob/develop/components/tools/OmeroWeb/omeroweb/settings.py#L475, but that is no longer true https://github.com/openmicroscopy/openmicroscopy/blob/develop/components/tools/OmeroWeb/omeroweb/api/views.py#L56.

@manics

manics commented Jan 17, 2017

Copy link
Copy Markdown
Member

@will-moore The real world. We had a load of problems with the IDR due to multi-layered proxies, so just wanted to be sure this would work.

@will-moore

Copy link
Copy Markdown
Member Author

Failing test test_dataset_images due to image_url not being ignored.

        assert_objects(conn, rsp['data'], [orphaned], dtype='Image',
>                      opts={'load_pixels': True})

test/integration/test_api_images.py:150: 

Need to handle this as in test_api_projects.py

Subclasses can configure self.urls to specify urls to add.
See ProjectsView urls as example
"""
# if self.urls is None:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

remove if no longer needed


# Urls to add to marshalled object. See ProjectsView for more details
urls = {
'dataset_url': {'name': 'api_dataset',

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

maybe keep the same order for the url list in each view
i.e. retrieval of children then object e.g. datasets/project

@atarkowska atarkowska Jan 18, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

in Python order in dict is not guaranteed (unless you use https://docs.python.org/2.7/library/collections.html#collections.OrderedDict), why order matters?

@jburel jburel Jan 18, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I found it easier to read when reviewing the code
This is mainly for review later on when we get back to it

@will-moore will-moore mentioned this pull request Jan 18, 2017
@will-moore

Copy link
Copy Markdown
Member Author

As discussed in today's web meeting, we'll use the url: prefix for urls e.g. url:datasets since this seems compatible with omero: prefix. Updated in commits above.

"""Build an absolute url using client response url."""
response = client.request()
# http://testserver/webclient/
webclint_url = response.url

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

typo: I think you mean webclient_url

@will-moore

Copy link
Copy Markdown
Member Author

@jburel fixed typo, thanks

@will-moore

Copy link
Copy Markdown
Member Author

@jburel

jburel commented Jan 20, 2017

Copy link
Copy Markdown
Member

@will-moore I guess your comment is for #5023

@will-moore will-moore closed this Jan 20, 2017
@will-moore will-moore reopened this Jan 20, 2017
@will-moore will-moore closed this Jan 20, 2017
@will-moore will-moore reopened this Jan 20, 2017
@jburel

jburel commented Jan 23, 2017

Copy link
Copy Markdown
Member

Thanks @will-moore. Merging

@jburel
jburel merged commit 2b4e9f8 into ome:develop Jan 23, 2017
@jburel jburel added this to the 5.3.0 milestone Mar 29, 2017
@will-moore
will-moore deleted the api_urls_in_json branch February 18, 2019 04:09
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.

4 participants