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

nepal_dhangadhi: Don't follow redirect to https://admin.ims.susasan.org/login #644

Merged
merged 3 commits into from
Mar 2, 2021

Conversation

jpmckinney
Copy link
Member

No description provided.

Signed-off-by: Yohanna Lisnichuk <yohanitalisnichuk@gmail.com>
@yolile
Copy link
Member

yolile commented Mar 2, 2021

@jpmckinney I've added one change now as we were missing one file with the current approach

@jpmckinney
Copy link
Member Author

I'm not clear what the parse method is doing?

The problem I'm trying to solve is that https://admin.ims.susasan.org/ocds/json/dhangadhi-2077-78.json redirects to https://admin.ims.susasan.org/login and will never succeed.

curl -I https://admin.ims.susasan.org/ocds/json/dhangadhi-2077-78.json

If I understand correctly, the new method will retry the 302, but allow the redirect, which will be to a bad page.

@yolile
Copy link
Member

yolile commented Mar 2, 2021

and will never succeed.

Actually, it only redirects the first time, but then, if we try again, it works! (and doesn't redirect anymore either)

@jpmckinney
Copy link
Member Author

Weird, I can run curl many times and it's always the same. I'll try the updated code.

@jpmckinney
Copy link
Member Author

Hmm, for me, dhangadhi-2077-78.json downloads as an HTML file, which we don't want.

Signed-off-by: Yohanna Lisnichuk <yohanitalisnichuk@gmail.com>
@yolile
Copy link
Member

yolile commented Mar 2, 2021

you are right :( I added a Caveats though, feel free to edit it if you want

@jpmckinney jpmckinney merged commit a3cc3f4 into main Mar 2, 2021
@jpmckinney jpmckinney deleted the fix-nepal_dhanghadi branch March 2, 2021 21:52
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.

2 participants