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

url should be local #1

Open
xristy opened this issue Feb 4, 2018 · 17 comments
Open

url should be local #1

xristy opened this issue Feb 4, 2018 · 17 comments
Assignees

Comments

@xristy
Copy link
Contributor

xristy commented Feb 4, 2018

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
Copy link
Contributor Author

xristy commented Feb 5, 2018

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)

@xristy xristy reopened this Feb 5, 2018
@berger-n
Copy link
Contributor

berger-n commented Feb 5, 2018

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

@xristy
Copy link
Contributor Author

xristy commented Feb 5, 2018

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
Copy link
Contributor

berger-n commented Feb 5, 2018

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
Copy link
Contributor Author

xristy commented Feb 5, 2018

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
Copy link
Contributor

eroux commented Feb 5, 2018

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
Copy link
Contributor Author

xristy commented Feb 5, 2018

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
Copy link
Contributor

berger-n commented Feb 6, 2018

this sounds very elegant to me
working on it

@berger-n
Copy link
Contributor

berger-n commented Feb 6, 2018

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

@xristy
Copy link
Contributor Author

xristy commented Feb 6, 2018

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
Copy link
Contributor

berger-n commented Feb 6, 2018

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

@eroux
Copy link
Contributor

eroux commented Feb 6, 2018

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
Copy link
Contributor

berger-n commented Feb 7, 2018

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

image

@berger-n
Copy link
Contributor

berger-n commented Feb 7, 2018

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

@berger-n
Copy link
Contributor

berger-n commented Feb 7, 2018

should be ok to build now

@xristy
Copy link
Contributor Author

xristy commented Feb 7, 2018

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
Copy link
Contributor

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

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

No branches or pull requests

3 participants