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

Update setup readme #1134

Merged
merged 4 commits into from
Oct 9, 2023
Merged

Update setup readme #1134

merged 4 commits into from
Oct 9, 2023

Conversation

ztefanie
Copy link
Member

@ztefanie ztefanie commented Oct 6, 2023

Updated some of the instructions to setup the project

11. Create an admin account using `./gradlew run --args="create-admin <project> <role> <email> <password> <region>"`
12. Take a look at the martin endpoints: [http://localhost:5002/tiles/accepting_stores/index.json](http://localhost:5002/tiles/accepting_stores/index.json) and [http://localhost:5002/tiles/accepting_stores/rpc/index.json](http://localhost:5002/tiles/accepting_stores/rpc/index.json). The data shown on the map is fetched from a hardcoded url and is not using the data from the local martin!
13. Take a look at the style by viewing the test map: [http://localhost:5002](http://localhost:5002)
14. Take a look at the backend: [http://localhost:8000](http://localhost:8000) (The public version is available at
Copy link
Member Author

Choose a reason for hiding this comment

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

Those links on ports 5002 and 8000 are not working for me. They all return 404 Not found error, except http://localhost:5002/tiles/accepting_stores/rpc/index.json it returns can not parse "accepting_stores" to a i16.

Is this also the case for you? If so, what would be the correct links? If not, how can I fix this?

Copy link
Contributor

@sarahsporck sarahsporck Oct 6, 2023

Choose a reason for hiding this comment

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

Okay, I did some evaluating. So the links currently don't work and that is correct, I guess. I guess @maxammann just forgot to remove them when he updated the ./docker/nginx*.config. As in earlier days there was one config for the development setup and one for staging. Now, there is only one config which is used for both. Therefore, at least that's what I guess what happened here, the map and style and other information have been stripped from nginx and the docs haven't been updated.
It is possible to readd the links. But for that I'd ask @maxammann or @f1sh1918, whether this would have other implications.

Copy link
Member

Choose a reason for hiding this comment

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

Calling http://localhost:8000 gives the root of the backend that is a 404 currently. This is expected. We could add a route in the backend to return some text or HTML.

A 404 on http://localhost:5002` is also expected. The root of the martin tile server returns a 404.

It is unexpected that getting the tilejson for the accepting stores return an error through this link http://localhost:5002/tiles/accepting_stores/rpc/index.json. I can not make sense of the error without digging deeper or setting it up.

Not sure about the changes in the docker setup. I think the URLs above should work as you need them for a local development setup.

Copy link
Contributor

Choose a reason for hiding this comment

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

Then I'll propose a PR to readd support for those links and create another ticket for digging into the issue with http://localhost:5002/tiles/accepting_stores/rpc/index.json

Copy link
Member Author

@ztefanie ztefanie Oct 9, 2023

Choose a reason for hiding this comment

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

So i will just add a hint to the docs that it is supposed these urls return a 404 or should I just keep it as it is?

Copy link
Contributor

Choose a reason for hiding this comment

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

Keep it as it is, I'll fix it in #1141

Copy link
Contributor

Choose a reason for hiding this comment

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

@ztefanie ahh. before I miss it. localhost:8000 is supposed to not work as this port is not even mirrored by docker in the docker compose. So you could just remove 14. ...

docs/development-setup.md Outdated Show resolved Hide resolved
11. Create an admin account using `./gradlew run --args="create-admin <project> <role> <email> <password> <region>"`
12. Take a look at the martin endpoints: [http://localhost:5002/tiles/accepting_stores/index.json](http://localhost:5002/tiles/accepting_stores/index.json) and [http://localhost:5002/tiles/accepting_stores/rpc/index.json](http://localhost:5002/tiles/accepting_stores/rpc/index.json). The data shown on the map is fetched from a hardcoded url and is not using the data from the local martin!
13. Take a look at the style by viewing the test map: [http://localhost:5002](http://localhost:5002)
14. Take a look at the backend: [http://localhost:8000](http://localhost:8000) (The public version is available at
Copy link
Contributor

@sarahsporck sarahsporck Oct 6, 2023

Choose a reason for hiding this comment

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

Okay, I did some evaluating. So the links currently don't work and that is correct, I guess. I guess @maxammann just forgot to remove them when he updated the ./docker/nginx*.config. As in earlier days there was one config for the development setup and one for staging. Now, there is only one config which is used for both. Therefore, at least that's what I guess what happened here, the map and style and other information have been stripped from nginx and the docs haven't been updated.
It is possible to readd the links. But for that I'd ask @maxammann or @f1sh1918, whether this would have other implications.

@ztefanie ztefanie merged commit f63fd79 into main Oct 9, 2023
2 checks passed
@ztefanie ztefanie deleted the Adjust-setup-readme branch October 9, 2023 12:22
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