Make ping_google use https for the sitemap URL

[Julian Wachholz]

The django.contrib.sitemaps.ping_google function currently does not allow modification of the protocol and will always send Google a "​http://" link.

Maybe we should also add a way to supply an absolute URL (including a protocol) to the function so we can run this from CLI without having contrib.sites installed.

[Danilo Bargen]

Why would you want to change the protocol?

Anyways, there actually is a way to change the ping URL: ​https://github.com/django/django/blob/master/django/contrib/sitemaps/__init__.py#L17 It probably needs a doc update though.

[Burhan Khalid]

This does not change the scheme, it just changes the URL. The scheme is hard coded (see ​https://github.com/django/django/blob/master/django/contrib/sitemaps/__init__.py#L42). In addition, this relies on django.contrib.sites, which is not enabled by default.

I would like to propose a fix to this which adds two optional commands to the ping_google method:

• is_secure, which will flag for ​https:// (defaults to False)
• site_domain which can be used to pass in the domain if django.contrib.sites is not used.

Here are my proposed changes: ​https://github.com/burhan/django/tree/ticket_23829

[Danilo Bargen]

Ah, you're talking about the scheme of the sitemap URL... Would you mind creating a pull request against master? That would make review easier :)

Here's my (now incomplete) pull request with doc updates: ​https://github.com/django/django/pull/3527

[Burhan Khalid]

Pull request done, and I am getting some help in writing tests as well.

[Sanyam Khurana]

PR is up for a review: ​https://github.com/django/django/pull/10651

[Sanyam Khurana]

I've addressed the reviews. Can you please take another pass?

[Carlton Gibson]

Reviewing, the PR looks almost ready to go.

Posting here for input: it's late-2018, I propose the is_secure flag default to True, i.e. we default to https://....

• This would be a slight breaking change for anyone using this to send http URLs — they'd need to pass the is_secure flag.
• But, presumably the vast majority (and best practice now) wouldn't need to set the flag at all. (On balance less key-strokes)

Thoughts/objections?

Update: After discussion on the PR, defaulting to https and having a --use-http flag was implemented. Unless there are objections we'll go with this.

[Carlton Gibson]

Bar an adjustment to AUTHORS.txt, this looks good to me.

[Carlton Gibson]

Adam argues on the PR that we should go through Deprecation for the breaking change here. Pause while we sort that out.

[Carlton Gibson]

For me, calling out the need to add the --use_http flag is enough. (Some people will need to add it sure, but everyone benefits from the changes.) I'll mark it RFC so we can ensure it's at least considered for v2.2. 

[Simon Charette]

Both flags should be changed to use - instead of underscores for separators to be coherent with ​the existing ones.

That means --site-domain and --use-http.

[Sanyam Khurana]

Just need help on this one, ​https://github.com/django/django/pull/10651#issuecomment-453040019

Addressed all other reviews.

[Tim Graham]

As I noted on the PR, the ticket reporter talks about "run ping_google from CLI without having contrib.sites installed." but you can't run a management command without it's app installed. Unless someone has a use case for it, I'm in favor of deferring the site domain changes for a separate ticket (if at all).

​PR for making the ping_google command and function use https for the sitemap URL. I think that's straightforward.

[Adam Johnson]

+1 to your proposal Tim. Well spotted.

[Tim Graham <timograham@…>]

In 76d31be:

Refs #23829 -- Made ping_google command/function use https for the sitemap URL.