You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by Daniel Sahlberg <da...@gmail.com> on 2022/01/05 12:45:41 UTC

Re: svn commit: r1896485 - /subversion/site/staging/download.html

Den tis 28 dec. 2021 kl 21:14 skrev <ds...@apache.org>:

> Author: dsahlberg
> Date: Tue Dec 28 20:14:17 2021
> New Revision: 1896485
>
> URL: http://svn.apache.org/viewvc?rev=1896485&view=rev
> Log:
> In site/staging: The ASF download script currently return the same site
> for both "http" and "backup" (after the migration to dlcdn.a.o).
>
> * download.html: Avoid displaying the same URL twice in the <select>
>
> Modified:
>     subversion/site/staging/download.html
>
> Modified: subversion/site/staging/download.html
> URL:
> http://svn.apache.org/viewvc/subversion/site/staging/download.html?rev=1896485&r1=1896484&r2=1896485&view=diff
>
> ==============================================================================
> --- subversion/site/staging/download.html (original)
> +++ subversion/site/staging/download.html Tue Dec 28 20:14:17 2021
> @@ -73,9 +73,9 @@ Other mirrors:
>    [if-any ftp]
>      [for ftp]<option value="[ftp]"[is ftp preferred]
> selected="selected"[end]>[ftp]</option>[end]
>    [end]
> -  [if-any backup]
> +  [if-any backup][is backup http][else][# Only show backup if different
> from http ]
>      [for backup]<option value="[backup]"[is backup preferred]
> selected="selected"[end]>[backup] (backup)</option>[end]
> -  [end]
> +  [end][end]
>  </select>
>  <input type="submit" value="Change" />
>  </form>
>

The reasoning behind this change is described in the log message but I'm
not happy about it for a few reasons:
* [http] and [backup] are arrays, so comparing these are probably not a
good idea. I can't come up with anything reasonably simple though.
* The lua version of EZT doesn't seem to support [else] at all. The fact
that it works for me is probably purely accidental.

I have a strong feeling that the download page will not work in case
different URLs are returned from the "closer.lua" script. Please check if
you are getting different lists in [http] and [backup] from
https://subversion-staging.apache.org/download.cgi?as_json=1 (I'm getting
https://dlcdn.apache.org/ on both of these). In that case check if the
<select> looks reasonable at
https://subversion-staging.apache.org/download.cgi. Also compare with our
published download page at https://subversion.apache.org/download.cgi.

I'm inclined to revert this change and live with the fact that the download
page can look slightly silly (displaying dlcdn.a.o as both the main and the
backup mirror) until ASF Infra check on closer.lua (or someone else find a
more clever way of writing the EZT comparison code).

Kind regards,
Daniel

Re: svn commit: r1896485 - /subversion/site/staging/download.html

Posted by Nathan Hartman <ha...@gmail.com>.
On Thu, Jan 6, 2022 at 1:17 AM Nathan Hartman <ha...@gmail.com> wrote:
>
> On Wed, Jan 5, 2022 at 1:03 PM Daniel Sahlberg
> <da...@gmail.com> wrote:
> >
> > I think we should now merge staging => publish, everything looks good to me. Do you want to check and do the honors?
>
> Sure. I will check it one more time in the morning and then do it...

Done in r1896755

Cheers,
Nathan

Re: svn commit: r1896485 - /subversion/site/staging/download.html

Posted by Nathan Hartman <ha...@gmail.com>.
On Thu, Jan 6, 2022 at 1:17 AM Nathan Hartman <ha...@gmail.com> wrote:
>
> On Wed, Jan 5, 2022 at 1:03 PM Daniel Sahlberg
> <da...@gmail.com> wrote:
> >
> > I think we should now merge staging => publish, everything looks good to me. Do you want to check and do the honors?
>
> Sure. I will check it one more time in the morning and then do it...

Done in r1896755

Cheers,
Nathan

Re: svn commit: r1896485 - /subversion/site/staging/download.html

Posted by Nathan Hartman <ha...@gmail.com>.
On Wed, Jan 5, 2022 at 1:03 PM Daniel Sahlberg
<da...@gmail.com> wrote:
>
> Den ons 5 jan. 2022 kl 16:22 skrev Nathan Hartman <ha...@gmail.com>:
>> So, I think it might be a good idea to revert this change and let
>> Infra manage how they supply those arrays; if it seems as though
>> they're providing an incorrect listing, it should be fixed on their
>> end because that will affect all projects, not just Subversion.
>
>
> Done, in r1896725.

Thanks!

>> The removal of "You may consult the complete list of mirrors" link
>> should *not* be reverted. That page is now a 404.
>
>
> That was a separate commit (for easy reverting :-) ).
>
> I think we should now merge staging => publish, everything looks good to me. Do you want to check and do the honors?

Sure. I will check it one more time in the morning and then do it...

Cheers,
Nathan

Re: svn commit: r1896485 - /subversion/site/staging/download.html

Posted by Nathan Hartman <ha...@gmail.com>.
On Wed, Jan 5, 2022 at 1:03 PM Daniel Sahlberg
<da...@gmail.com> wrote:
>
> Den ons 5 jan. 2022 kl 16:22 skrev Nathan Hartman <ha...@gmail.com>:
>> So, I think it might be a good idea to revert this change and let
>> Infra manage how they supply those arrays; if it seems as though
>> they're providing an incorrect listing, it should be fixed on their
>> end because that will affect all projects, not just Subversion.
>
>
> Done, in r1896725.

Thanks!

>> The removal of "You may consult the complete list of mirrors" link
>> should *not* be reverted. That page is now a 404.
>
>
> That was a separate commit (for easy reverting :-) ).
>
> I think we should now merge staging => publish, everything looks good to me. Do you want to check and do the honors?

Sure. I will check it one more time in the morning and then do it...

Cheers,
Nathan

Re: svn commit: r1896485 - /subversion/site/staging/download.html

Posted by Daniel Sahlberg <da...@gmail.com>.
Den ons 5 jan. 2022 kl 16:22 skrev Nathan Hartman <hartman.nathan@gmail.com
>:

> Interesting. On publish, previously the "Other mirrors" list was
> showing me dlcdn.a.o for both the main and backup.
>
> As of right now, I am getting two options on publish:
> https://dlcdn.apache.org/
> https://downloads.apache.org/ (backup)
>
> While on staging, I am only getting
> https://dlcdn.apache.org/
>
> So, I think it might be a good idea to revert this change and let
> Infra manage how they supply those arrays; if it seems as though
> they're providing an incorrect listing, it should be fixed on their
> end because that will affect all projects, not just Subversion.
>

Done, in r1896725.


> The removal of "You may consult the complete list of mirrors" link
> should *not* be reverted. That page is now a 404.
>

That was a separate commit (for easy reverting :-) ).

I think we should now merge staging => publish, everything looks good to
me. Do you want to check and do the honors?

/Daniel

Re: svn commit: r1896485 - /subversion/site/staging/download.html

Posted by Daniel Sahlberg <da...@gmail.com>.
Den ons 5 jan. 2022 kl 16:22 skrev Nathan Hartman <hartman.nathan@gmail.com
>:

> Interesting. On publish, previously the "Other mirrors" list was
> showing me dlcdn.a.o for both the main and backup.
>
> As of right now, I am getting two options on publish:
> https://dlcdn.apache.org/
> https://downloads.apache.org/ (backup)
>
> While on staging, I am only getting
> https://dlcdn.apache.org/
>
> So, I think it might be a good idea to revert this change and let
> Infra manage how they supply those arrays; if it seems as though
> they're providing an incorrect listing, it should be fixed on their
> end because that will affect all projects, not just Subversion.
>

Done, in r1896725.


> The removal of "You may consult the complete list of mirrors" link
> should *not* be reverted. That page is now a 404.
>

That was a separate commit (for easy reverting :-) ).

I think we should now merge staging => publish, everything looks good to
me. Do you want to check and do the honors?

/Daniel

Re: svn commit: r1896485 - /subversion/site/staging/download.html

Posted by Nathan Hartman <ha...@gmail.com>.
On Wed, Jan 5, 2022 at 7:45 AM Daniel Sahlberg
<da...@gmail.com> wrote:
>
> Den tis 28 dec. 2021 kl 21:14 skrev <ds...@apache.org>:
>>
>> Author: dsahlberg
>> Date: Tue Dec 28 20:14:17 2021
>> New Revision: 1896485
>>
>> URL: http://svn.apache.org/viewvc?rev=1896485&view=rev
>> Log:
>> In site/staging: The ASF download script currently return the same site
>> for both "http" and "backup" (after the migration to dlcdn.a.o).
>>
>> * download.html: Avoid displaying the same URL twice in the <select>
>>
>> Modified:
>>     subversion/site/staging/download.html
>>
>> Modified: subversion/site/staging/download.html
>> URL: http://svn.apache.org/viewvc/subversion/site/staging/download.html?rev=1896485&r1=1896484&r2=1896485&view=diff
>> ==============================================================================
>> --- subversion/site/staging/download.html (original)
>> +++ subversion/site/staging/download.html Tue Dec 28 20:14:17 2021
>> @@ -73,9 +73,9 @@ Other mirrors:
>>    [if-any ftp]
>>      [for ftp]<option value="[ftp]"[is ftp preferred] selected="selected"[end]>[ftp]</option>[end]
>>    [end]
>> -  [if-any backup]
>> +  [if-any backup][is backup http][else][# Only show backup if different from http ]
>>      [for backup]<option value="[backup]"[is backup preferred] selected="selected"[end]>[backup] (backup)</option>[end]
>> -  [end]
>> +  [end][end]
>>  </select>
>>  <input type="submit" value="Change" />
>>  </form>
>
>
> The reasoning behind this change is described in the log message but I'm not happy about it for a few reasons:
> * [http] and [backup] are arrays, so comparing these are probably not a good idea. I can't come up with anything reasonably simple though.
> * The lua version of EZT doesn't seem to support [else] at all. The fact that it works for me is probably purely accidental.
>
> I have a strong feeling that the download page will not work in case different URLs are returned from the "closer.lua" script. Please check if you are getting different lists in [http] and [backup] from https://subversion-staging.apache.org/download.cgi?as_json=1 (I'm getting https://dlcdn.apache.org/ on both of these). In that case check if the <select> looks reasonable at https://subversion-staging.apache.org/download.cgi. Also compare with our published download page at https://subversion.apache.org/download.cgi.
>
> I'm inclined to revert this change and live with the fact that the download page can look slightly silly (displaying dlcdn.a.o as both the main and the backup mirror) until ASF Infra check on closer.lua (or someone else find a more clever way of writing the EZT comparison code).


Interesting. On publish, previously the "Other mirrors" list was
showing me dlcdn.a.o for both the main and backup.

As of right now, I am getting two options on publish:
https://dlcdn.apache.org/
https://downloads.apache.org/ (backup)

While on staging, I am only getting
https://dlcdn.apache.org/

So, I think it might be a good idea to revert this change and let
Infra manage how they supply those arrays; if it seems as though
they're providing an incorrect listing, it should be fixed on their
end because that will affect all projects, not just Subversion.

The removal of "You may consult the complete list of mirrors" link
should *not* be reverted. That page is now a 404.

Cheers,
Nathan

Re: svn commit: r1896485 - /subversion/site/staging/download.html

Posted by Nathan Hartman <ha...@gmail.com>.
On Wed, Jan 5, 2022 at 7:45 AM Daniel Sahlberg
<da...@gmail.com> wrote:
>
> Den tis 28 dec. 2021 kl 21:14 skrev <ds...@apache.org>:
>>
>> Author: dsahlberg
>> Date: Tue Dec 28 20:14:17 2021
>> New Revision: 1896485
>>
>> URL: http://svn.apache.org/viewvc?rev=1896485&view=rev
>> Log:
>> In site/staging: The ASF download script currently return the same site
>> for both "http" and "backup" (after the migration to dlcdn.a.o).
>>
>> * download.html: Avoid displaying the same URL twice in the <select>
>>
>> Modified:
>>     subversion/site/staging/download.html
>>
>> Modified: subversion/site/staging/download.html
>> URL: http://svn.apache.org/viewvc/subversion/site/staging/download.html?rev=1896485&r1=1896484&r2=1896485&view=diff
>> ==============================================================================
>> --- subversion/site/staging/download.html (original)
>> +++ subversion/site/staging/download.html Tue Dec 28 20:14:17 2021
>> @@ -73,9 +73,9 @@ Other mirrors:
>>    [if-any ftp]
>>      [for ftp]<option value="[ftp]"[is ftp preferred] selected="selected"[end]>[ftp]</option>[end]
>>    [end]
>> -  [if-any backup]
>> +  [if-any backup][is backup http][else][# Only show backup if different from http ]
>>      [for backup]<option value="[backup]"[is backup preferred] selected="selected"[end]>[backup] (backup)</option>[end]
>> -  [end]
>> +  [end][end]
>>  </select>
>>  <input type="submit" value="Change" />
>>  </form>
>
>
> The reasoning behind this change is described in the log message but I'm not happy about it for a few reasons:
> * [http] and [backup] are arrays, so comparing these are probably not a good idea. I can't come up with anything reasonably simple though.
> * The lua version of EZT doesn't seem to support [else] at all. The fact that it works for me is probably purely accidental.
>
> I have a strong feeling that the download page will not work in case different URLs are returned from the "closer.lua" script. Please check if you are getting different lists in [http] and [backup] from https://subversion-staging.apache.org/download.cgi?as_json=1 (I'm getting https://dlcdn.apache.org/ on both of these). In that case check if the <select> looks reasonable at https://subversion-staging.apache.org/download.cgi. Also compare with our published download page at https://subversion.apache.org/download.cgi.
>
> I'm inclined to revert this change and live with the fact that the download page can look slightly silly (displaying dlcdn.a.o as both the main and the backup mirror) until ASF Infra check on closer.lua (or someone else find a more clever way of writing the EZT comparison code).


Interesting. On publish, previously the "Other mirrors" list was
showing me dlcdn.a.o for both the main and backup.

As of right now, I am getting two options on publish:
https://dlcdn.apache.org/
https://downloads.apache.org/ (backup)

While on staging, I am only getting
https://dlcdn.apache.org/

So, I think it might be a good idea to revert this change and let
Infra manage how they supply those arrays; if it seems as though
they're providing an incorrect listing, it should be fixed on their
end because that will affect all projects, not just Subversion.

The removal of "You may consult the complete list of mirrors" link
should *not* be reverted. That page is now a 404.

Cheers,
Nathan