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

x/pkgsite: automatically fetch a requested package if it does not exist on pkg.go.dev #37002

Closed
natefinch opened this issue Feb 3, 2020 · 32 comments

Comments

@natefinch
Copy link
Contributor

@natefinch natefinch commented Feb 3, 2020

Currently, if you request package docs from pkg.go.dev for a package that hasn't been sent to the go proxy yet, then it just returns 404.

Requesting a url from pkg.go.dev should do whatever "add your package" does, if that's required to view the docs, because clearly I want to view the docs and the server knows exactly what step is missing to allow me to view the docs, so just do it, please.

By returning a 404 page, it looks like there's a bug in the website. The suggested "how to add your package" requires some arcane steps (I presume? it's not actually spelled out) that simply shouldn't be necessary.

See #36986 for an example of someone reporting a "bug" because a package didn't show up.

This is actually super common when using godoc.org in my experience, as I will often want to view the docs on godoc.org as I'm writing the code, to see how they look there. I'm sure others will want to do the same on pkg.go.dev.

This can also easily happen if most people in your company use a corporate proxy, so the code isn't in go's proxy yet, but then you decide you want to view the docs and you're just used to doing it on pkg.go.dev (which I do all the time for random go packages on godoc.org... rather than download them and view the docs locally, I view 99% of docs directly on godoc.org).

@julieqiu julieqiu changed the title go.dev: requesting unpopulated docs should generate them go.dev: automatically fetch a requested package if it does not exist on pkg.go.dev Feb 4, 2020
@gopherbot gopherbot added this to the Unreleased milestone Feb 6, 2020
@hunterloftis
Copy link

@hunterloftis hunterloftis commented Feb 7, 2020

I remember the "magic" of having my open-source projects auto-doc'd for me. Someone had to point out to me that I no longer needed to build my own markdown-generating toolchain for docs - instead, they would "just exist" and even be searchable, alongside major, important packages.

I believe this suggestion aligns with that beginner-friendly "it just works" philosophy that has long been a part of the language and community.

@ghost
Copy link

@ghost ghost commented Feb 8, 2020

If immediate fetch is not possible for some reason, then add a "request documentation" button to the 404 page. Clicking the button kicks of whatever needs to be done and shows UI explaining when the documentation will be available.

@deeper-x
Copy link

@deeper-x deeper-x commented Feb 27, 2020

I'm joining this thread because I consider this issue a detrimental change in the language.
The way godoc.org works is how things should be, as Andrew Gerrand says

[...] it should be coupled to the code itself so the documentation evolves along with the code. The easier it is for programmers to produce good documentation, the better for everyone

Plain and simple: in Go, code & documentation are tightly coupled by default. No surprises expected, because publishing your code you're publishing the documentation on godoc.org as well.

What you quietly introduced with pkg.go.dev and mention there breaks this natural flow introducing a weird exception, silently assuming that now could exist public code-without-documentation.
Furthermore, suggesting a workaround like forcing a fake download in order to trigger the documentation generation, cannot be considered a stable solution.

gopherbot pushed a commit to golang/pkgsite that referenced this issue Jun 15, 2020
The logic for GetVersionMap is changed so that an explicit module path
is now required. All logic is also now the same for all
requestedVersions (as opposed to a special case for latest).

This function is only used by the frontend to check if a version exists
as part of a frontend fetch, and this behavior makes more sense for that
use case.

Updates golang/go#36811
Updates golang/go#37002
Updates golang/go#37106

Change-Id: I415a2730daa6edc023f80c0c615521047311f35b
Reviewed-on: https://team-review.git.corp.google.com/c/golang/discovery/+/744833
Reviewed-by: Jonathan Amsterdam <jba@google.com>
gopherbot pushed a commit to golang/pkgsite that referenced this issue Jun 15, 2020
The frontend server is not initiated with a queue. The frontend task
queue will be used to support frontend fetches.

frontend.FetchAndUpdateState is added, which is a copy of
worker.FetchAndUpdateState for use in testing and locally.

Updates golang/go#36811
Updates golang/go#37002
Updates golang/go#37106

Change-Id: I41922d30462d2623a061aa1f207bb2b39f7b54e2
Reviewed-on: https://team-review.git.corp.google.com/c/golang/discovery/+/743102
Reviewed-by: Jonathan Amsterdam <jba@google.com>
gopherbot pushed a commit to golang/pkgsite that referenced this issue Jun 15, 2020
This column will be used to display the canonical module path when a
frontend fetch results in fetching an alternative module path.

Updates golang/go#36811
Updates golang/go#37002
Updates golang/go#37106

Change-Id: Ie8ee6ba64ea799d264c1079c345e37d2e073da38
Reviewed-on: https://team-review.git.corp.google.com/c/golang/discovery/+/751268
Reviewed-by: Jonathan Amsterdam <jba@google.com>
gopherbot pushed a commit to golang/pkgsite that referenced this issue Jun 15, 2020
The version_map.go_mod_path column is now populated in
UpsertVersionMap.

Updates golang/go#36811
Updates golang/go#37002
Updates golang/go#37106

Change-Id: I4e474d28bee67caf7625045082df46f93394f3f6
Reviewed-on: https://team-review.git.corp.google.com/c/golang/discovery/+/751269
Reviewed-by: Jonathan Amsterdam <jba@google.com>
gopherbot pushed a commit to golang/pkgsite that referenced this issue Jun 15, 2020
FetchAndUpdateState in internal/frontend and internal/worker now
insert the GoModPath field on VersionMap.

Updates golang/go#36811
Updates golang/go#37002
Updates golang/go#37106

Change-Id: Idfce6d685d24d915608acc1748a84b9db5312ae4
Reviewed-on: https://team-review.git.corp.google.com/c/golang/discovery/+/751270
CI-Result: Cloud Build <devtools-proctor-result-processor@system.gserviceaccount.com>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
gopherbot pushed a commit to golang/pkgsite that referenced this issue Jun 15, 2020
A fetch endpoint is added to the frontend, which will queue a module to
be enqueued by the worker, if it doesn't already exist in the database.

After enqueuing, the fetch handler will poll the version_map table at a
constant rate, until the path returns or the request times out.

If the request fails, a corresponding statusCode and responseText will
be returned to be displayed to the user.

Updates golang/go#36811
Updates golang/go#37002
Updates golang/go#37106

Change-Id: Ic2e20146dc626bf296db05bc2abbfb50d6fd7991
Reviewed-on: https://team-review.git.corp.google.com/c/golang/discovery/+/743103
CI-Result: Cloud Build <devtools-proctor-result-processor@system.gserviceaccount.com>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
gopherbot pushed a commit to golang/pkgsite that referenced this issue Jun 15, 2020
When a path page 404s, it will now render the notfound.tmpl page (once
the frontend-fetch feature flag is on), which provides a button for the
user to make a request to fetch the package.

Updates golang/go#36811
Updates golang/go#37002
Updates golang/go#37106

Change-Id: I17fedd018435e8d3e51e2a2a4a972d3cf673df56
Reviewed-on: https://team-review.git.corp.google.com/c/golang/discovery/+/753606
CI-Result: Cloud Build <devtools-proctor-result-processor@system.gserviceaccount.com>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
gopherbot pushed a commit to golang/pkgsite that referenced this issue Jun 15, 2020
At the moment when a request for path@version 404s, but other versions
of the path exists, we will return an error message letting users know
that is the case. Once frontend fetch is live, the message will change
to provide users an option to fetch that version of the path.

This logic is moved to pathFoundAtLatestError.

Updates golang/go#36811
Updates golang/go#37002
Updates golang/go#37106

Change-Id: I1ad15ee13714a68b4b88fd353e43719fda0c0d31
Reviewed-on: https://team-review.git.corp.google.com/c/golang/discovery/+/754818
CI-Result: Cloud Build <devtools-proctor-result-processor@system.gserviceaccount.com>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
@julieqiu julieqiu changed the title go.dev: automatically fetch a requested package if it does not exist on pkg.go.dev x/pkgsite: automatically fetch a requested package if it does not exist on pkg.go.dev Jun 15, 2020
@gopherbot
Copy link

@gopherbot gopherbot commented Jul 28, 2020

Change https://golang.org/cl/245020 mentions this issue: internal/frontend: use StatusNotFoundInVersionMap for frontend fetch

gopherbot pushed a commit to golang/pkgsite that referenced this issue Jul 28, 2020
s.fetchModule is removed. The logic from that function is moved into
checkPossibleModulePaths, which is renamed from checkPossiblePaths for
clarity.

For golang/go#36811
For golang/go#37002

Change-Id: I8ac26ac13fcddc19582682bd5351b0b7a6a2858f
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/245021
Run-TryBot: Julie Qiu <julie@golang.org>
TryBot-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
gopherbot pushed a commit to golang/pkgsite that referenced this issue Jul 28, 2020
Previously, if the context deadline exceeded when calling GetVersionMap
in checkForPath, a 500 was returned. This is fixed so that a 408 is
returned instead.

For golang/go#36811
For golang/go#37002

Change-Id: I60d78fa0e76d5f3e4bb4dab34d66d73a45121955
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/245121
Run-TryBot: Julie Qiu <julie@golang.org>
TryBot-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
gopherbot pushed a commit to golang/pkgsite that referenced this issue Jul 28, 2020
Previously, we used http.StatusProcessing and http.StatusNoContent to
indicate when a row did not exist in the version_map table.

Neither of these status codes fit well, so our own error code is defined
for the case when a module path has not been processed yet:
statusNotFoundInVersionMap.

For golang/go#36811
For golang/go#37002

Change-Id: Ia55972bfbfb1950ad82254d8f1082dda052bf376
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/245020
Run-TryBot: Julie Qiu <julie@golang.org>
TryBot-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
gopherbot pushed a commit to golang/pkgsite that referenced this issue Jul 28, 2020
When a fetch request results in a 404, it's possible that the module
path will appear later in the index. Similarly, when a fetch request
results in a 480 or 500, it's possible that the error was transient or
has been fixed but not yet reprocessed.

In these cases, we should allow the user to refetch the path after the
taskIDChangeInterval has passed.

For golang/go#36811
For golang/go#37002

Change-Id: Ia4813140ca793b7b91d80bdc196f61af075766ca
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/244601
Reviewed-by: Jonathan Amsterdam <jba@google.com>
@gopherbot
Copy link

@gopherbot gopherbot commented Jul 29, 2020

Change https://golang.org/cl/245437 mentions this issue: content: alphabetize classes with prefix Fetch

@gopherbot
Copy link

@gopherbot gopherbot commented Jul 29, 2020

Change https://golang.org/cl/245402 mentions this issue: internal/worker: don't delete 40x results from version_map

@gopherbot
Copy link

@gopherbot gopherbot commented Jul 29, 2020

Change https://golang.org/cl/245401 mentions this issue: content: center text in h3.FetchMessage

gopherbot pushed a commit to golang/pkgsite that referenced this issue Jul 29, 2020
Class names with the prefix "Fetch" are alphabetized relative
to each other. No changes are made to the CSS.

For golang/go#36811
For golang/go#37002

Change-Id: I662829fd50f1e1634cc891ba50c4bf1d4f9ac0bd
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/245437
Reviewed-by: Jonathan Amsterdam <jba@google.com>
gopherbot pushed a commit to golang/pkgsite that referenced this issue Jul 29, 2020
At the moment, when module is fetched with a status 40x, it is deleted
from the database. However, we don't want to delete that module from
version_map, otherwise the response will never be returned to the user
for a frontend fetch.

Modules with status 40x are now deleted first, and then the result is
inserted into version_map and module_version_states.

For golang/go#36811
For golang/go#37002

Change-Id: I5eb9414c065bdc8364445edb293f21bbe870fa43
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/245402
Reviewed-by: Jonathan Amsterdam <jba@google.com>
gopherbot pushed a commit to golang/pkgsite that referenced this issue Jul 29, 2020
For golang/go#36811
For golang/go#37002

Change-Id: I3438a8d72617f3c7af88310469b4799a5539616e
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/245401
Reviewed-by: Jonathan Amsterdam <jba@google.com>
Reviewed-by: Jamal Carvalho <jamal@golang.org>
@gopherbot
Copy link

@gopherbot gopherbot commented Jul 29, 2020

Change https://golang.org/cl/245640 mentions this issue: internal/frontend: tweak response text for fetch requests

gopherbot pushed a commit to golang/pkgsite that referenced this issue Jul 29, 2020
The response text for different frontend fetch outcomes is
updated.

The code in fetchRequestStatusAndResponseText is also
refactored.

For golang/go#36811
For golang/go#37002

Change-Id: I175921acde5edc5cbf677eb1d1f23171b40318b9
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/245640
Run-TryBot: Julie Qiu <julie@golang.org>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
Reviewed-by: Jamal Carvalho <jamal@golang.org>
TryBot-Result: kokoro <noreply+kokoro@google.com>
@gopherbot
Copy link

@gopherbot gopherbot commented Jul 29, 2020

Change https://golang.org/cl/245646 mentions this issue: content: update text in fetch.tmpl for 404s

gopherbot pushed a commit to golang/pkgsite that referenced this issue Jul 30, 2020
For golang/go#36811
For golang/go#37002

Change-Id: I1853ecf2798e717589bb81ad0d749a3f814b55ec
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/245646
Reviewed-by: Jonathan Amsterdam <jba@google.com>
@gopherbot
Copy link

@gopherbot gopherbot commented Jul 30, 2020

Change https://golang.org/cl/245880 mentions this issue: internal/fronted: update text in fetch.tmpl

@gopherbot
Copy link

@gopherbot gopherbot commented Jul 30, 2020

Change https://golang.org/cl/245878 mentions this issue: internal/frontend: return 404 on invalid stdlib paths

gopherbot pushed a commit to golang/pkgsite that referenced this issue Jul 30, 2020
The text in fetch.tmpl is updated to provide clearer instructions.

Corresponding CSS is also adjusted for these text changes.

For golang/go#37002

Change-Id: Iae8b641eb2471172eb4465493fe83a4005bce6fc
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/245880
Run-TryBot: Julie Qiu <julie@golang.org>
Reviewed-by: Jamal Carvalho <jamal@golang.org>
gopherbot pushed a commit to golang/pkgsite that referenced this issue Jul 30, 2020
At the moment, pkg.go.dev/404 returns the following error message:

Package “404” could not be found, but you can view module “std” at
https://pkg.go.dev/mod/std.

This is because stdlib.Contains(404) returns true.

This error is fixed so that pkg.go.dev/404 just shows a normal 404
error, when frontend fetch is active.

For golang/go#37002

Change-Id: I787a10009294d105d3f92df19790dcd438123f8d
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/245878
Reviewed-by: Jonathan Amsterdam <jba@google.com>
@gopherbot
Copy link

@gopherbot gopherbot commented Jul 30, 2020

Change https://golang.org/cl/245897 mentions this issue: internal/frontend: improve validation of candidate module paths for fetch

gopherbot pushed a commit to golang/pkgsite that referenced this issue Jul 30, 2020
…etch

Additional source hosts are added to pre-filter invalid module paths.
Each path is also validated using module.CheckImportPath.

For golang/go#37002

Change-Id: I4090a915c023b4d84b9b139e74256c61ca183176
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/245897
Reviewed-by: Jonathan Amsterdam <jba@google.com>
@julieqiu
Copy link
Contributor

@julieqiu julieqiu commented Jul 30, 2020

Users can now request a package by clicking a button on the 404 page.

Here's an example:

Screen Shot 2020-07-30 at 4 03 01 PM

@criscola
Copy link

@criscola criscola commented Jan 21, 2021

Users can now request a package by clicking a button on the 404 page.

Is there any way to retrigger the request for a module? I've updated my module (same version, forgot small addition, no users are relying on my package) but I still see the older version.

@julieqiu
Copy link
Contributor

@julieqiu julieqiu commented Jan 21, 2021

@criscola you can either visit https://pkg.go.dev/<your-module-path>@master, or follow the instructions at https://go.dev/about#adding-a-package.

@criscola
Copy link

@criscola criscola commented Jan 24, 2021

@julieqiu I've tried but my godocs is still not getting updated to the latest commit.

@julieqiu
Copy link
Contributor

@julieqiu julieqiu commented Jan 25, 2021

@criscola - since this is a closed issue, could you please file a new issue at https://golang.org/s/pkgsite-feedback?

#43096 also seems related.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants