buda-base / blmp-prototype-flow

Prototype of BLMP client, implemented using flow.
0 stars 0 forks source link

url should be local #1

Open xristy opened 6 years ago

xristy commented 6 years ago

The urls at src/api/api.js line #'s 248 and 264 should be local so that blmp is using the local lds-pdi rather than the one at buda1. If the blmp is running on buda1 then it will automatically be using the lds-pdi on buda1.

Or perhaps some other configuration approach that allows to easily switch where the lds-pdi is.

xristy commented 6 years ago

Interesting solution. The README.md should be updated to explain the fallback strategy.

It might be helpful to display the lds-pdi URL so that the user knows what URL is being used.

I suppose experience will indicate whether a manual override will be helpful. I'm pretty confident that during development it might well be useful to be able to quickly switch between a known good lds-pdi install (perhaps at purl.bdrc.io) and a development install of lds-pdi.

as Élie observed elsewhere:

(as a general observation, it tends to be perceived as rude to close an item opened by someone else, the usual protocol is to ask the person who opened the issue if it's ok to be closed)

berger-n commented 6 years ago

(didn't mean to be rude of course - just wanted to test "autoclose" feature... ^^)

xristy commented 6 years ago

I just updated the blmp on AWS with the latest commit. I see that there is a url reported after I use the search for a resource.

In this instance the reported url is

http://buda1.bdrc.io:13280

which surprised me since I expected:

http://localhost:13280

would be returned from findLDSPDIhost which would be the desired result. Using buda1.bdrc.io on the AWS server does a DNS lookup that isn't needed.

It would be best to let the user know what lds-pdi server is going to be used before doing a search.

I am further persuaded that being able to select the lds-pdi server/url will be quite useful.

berger-n commented 6 years ago

regarding the issue on buda1, connection to http://localhost:13280/ fails... see error console : GET http://localhost:13280/resource/P1 net::ERR_CONNECTION_REFUSED I will try to get further details

I like your suggestion of a button for choosing which server to load data from (potentially displaying information on whether it can be reached) will do

xristy commented 6 years ago

I should have mentioned previously that I connected to blmp by visiting:

http://editor.bdrc.io

I'm ssh'd into buda1.bdrc.io and:

 curl -v http://localhost:13280

works fine.

Where are you seeing the error console? Are you ssh'd into buda1 on AWS?

eroux commented 6 years ago

Well, I have to say it's a bit hard for me to relate to the idea of the blmp auto-finding its configuration from a set of pre-defined possible server endpoints... what would be more useful, I think, would be for the blmp to read an optional /config.json that would contain this configuration, so that it can be different on each person's machine an on aws. In the long term it would also contain the auth data for instance (as at some point the blmp should only work when users are logged in).

xristy commented 6 years ago

I agree and that is why I've suggested that a list of reasonable urls to choose from would be helpful. I see your suggestion as adding the ability to enter a url of the user's choosing. It seems reasonable that these choices and urls can be saved in the proposed config.json.

berger-n commented 6 years ago

this sounds very elegant to me working on it

berger-n commented 6 years ago

BTW I was talking about error console in Chrome/Firefox inspector ( CTRL+SHIFT+I ) screenshot attached image

xristy commented 6 years ago

How idiotic of me!

I was thinking on the server not the client!

Of course the localhost refers to the system the browser is running on not to the server where the content was fetched from.

This makes it more interestingto allow the user to indicate where to contact lds-pdi

berger-n commented 6 years ago

you can now have a look added a config file and a menu to choose ldspdi address: image

eroux commented 6 years ago

Well, here we need a system to have both a default config that would be the current one and the ability for a user to override values without having to change the git file... what I had done in a previous project was to have config-defaults.json on git and a config.json not on git (in the .gitignore actually) for the user to overwrite the values. Does that seem reasonable? I guess another solution would be to use JavaScript instead of json as it allows more flexibility in the conf... anyway, there should be a way for users to have their local config without messing with a versionned file.

Also, less important but would it be possible to make it clearer that the field is a select (it doesn't look like users can interact with it)?

berger-n commented 6 years ago

et voilà added colored icon & subtitle to url + defaults config file & git-ignored local config cf https://material.io/icons/ image

image

berger-n commented 6 years ago

should fail in build environment though (must check if custom config file exists)

berger-n commented 6 years ago

should be ok to build now

xristy commented 6 years ago

editor.bdrc.io has been updated. This is an improvement. Thanks

It would be helpful to allow the user to enter a url of their choice, even if it is only for the current session, although maybe a cookie could be used. The issue is that the public/config-defaults.json resides on the server and the user may not have access to file to alter it. It may also be a testing situation and altering the defaults might not be appropriate

berger-n commented 6 years ago

you can now have a look I just added an empty field in the popup menu (which will be saved in a cookie)