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

Use My Society's Mapit when running locally #1994

Merged
merged 1 commit into from Oct 19, 2020

Conversation

leenagupte
Copy link
Contributor

GOV.UK hosts it's own version of Mapit. Unfortunately, that is only accessibly via the production
and test environments, not locally. Until we can access Mapit locally, add a work around to make it
possibly to manually test changes.

⚠️ This application is Continuously Deployed: ⚠️

  • Merged changes are automatically deployed to staging and production.

  • Make sure you follow the guidance for deployments before you merge.

  • Check your branch is being deployed in the Release app, after merging.

GOV.UK hosts it's own version of Mapit. Unfortunately, that is only accessibly via the production
and test environments, not locally. Until we can access Mapit locally, add a work around to make it
possibly to manually test changes.
@bevanloon bevanloon temporarily deployed to govuk-collec-add-work-a-zvuc2d October 19, 2020 09:24 Inactive
def mapit
return GdsApi.mapit unless Rails.env.development?

GdsApi::Mapit.new("https://mapit.mysociety.org/")
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible to do this on a global level? ie. inside of the GDS API adapter?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I kept it like that so it would still throw the same GdsApi errors, i.e. GdsApi::HTTPNotFound and GdsApi::HTTPClientError, otherwise that bit of the code will stop working.

Copy link
Contributor

Choose a reason for hiding this comment

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

Correct me if I'm wrong but wouldn't it still keep these errors as we'll be instantiating with the same class?

Anyways, I think, for now, we can localise the change until we have something better.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh! sorry! I mis-read! You meant, put this inside of the gem! 🤦 Yes, it should be possible to do that, but I only envisioned this as a stop-gap. There's a bigger problem around how do we access Mapit data locally.

Copy link
Contributor

@DilwoarH DilwoarH left a comment

Choose a reason for hiding this comment

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

Not a blocker but should we make this an environment variable?

@leenagupte
Copy link
Contributor Author

Not a blocker but should we make this an environment variable?

Could do, but I'm not convinced it's worth the effort at the moment.

@leenagupte leenagupte merged commit 5071d5b into master Oct 19, 2020
@leenagupte leenagupte deleted the add-work-around-for-local-mapit branch October 19, 2020 10:15
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.

None yet

3 participants