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

Issue sync progress is not accurate during synchronization #74

Closed
2 of 3 tasks
birkjernstrom opened this issue Apr 3, 2023 · 6 comments
Closed
2 of 3 tasks

Issue sync progress is not accurate during synchronization #74

birkjernstrom opened this issue Apr 3, 2023 · 6 comments
Assignees
Labels
bug Something isn't working

Comments

@birkjernstrom
Copy link
Member

birkjernstrom commented Apr 3, 2023

Some issues have emerged with the issue sync progress in onboarding. Likely due to some changes with issue edited signals and what the frontend expects in terms of the data.

Issues

  • Total count can change during sync
  • Can restart in the midst of syncing
  • (Existed before too) Includes PRs in the total count
@birkjernstrom birkjernstrom added bug Something isn't working onboarding labels Apr 3, 2023
@birkjernstrom birkjernstrom self-assigned this Apr 3, 2023
@birkjernstrom birkjernstrom removed their assignment Apr 3, 2023
@birkjernstrom birkjernstrom added this to the private-alpha milestone Apr 3, 2023
@hult hult self-assigned this Apr 11, 2023
@hult
Copy link
Contributor

hult commented Apr 11, 2023

If the total count changes during sync, that should be correctly reflected in the issue.synced event, right?

    await publish(
        "issue.synced",
        {
            "expected": repository.open_issues,

Also, any ideas on how to trigger a restart in the midst of syncing?

I've seen cases when the total disappears ("6 / issues fetched"), but can't make that happen now.

@hult
Copy link
Contributor

hult commented Apr 11, 2023

For the PRs in the total count, I propose one of two solutions:

  1. Either we decrease the expected number of issues to sync for every issue we skip (and every error),
  2. Or we expose the number of issues looked at (rather than successfully synced) to the client

I think I'm team 2, it'd be weird if the total number decreases as syncing progresses

@birkjernstrom
Copy link
Member Author

Not sure if it's what you meant with "trigger a restart amidst syncing", but you can trigger a new sync to ease development with

cd server/ && python -m scripts.github resync-issues
Typing on the go so it might not work out of the box, but the right direction

Re: Count of issues to sync. Hm, unless we can easily get and subtract the PR count (w.o unnecessary additional complexity), I'd propose a third option of just updating the design to be "syncing x / y issues and PRs"? WDYT?

@hult
Copy link
Contributor

hult commented Apr 11, 2023

Not sure if it's what you meant with "trigger a restart amidst syncing"

No, was referring to one of the tasks in the issue, and the fact that I don't really know what it means. :)

I'd propose a third option of just updating the design to be "syncing x / y issues and PRs"?

Yeah, that's what I've settled at (even if the syncing for the PRs don't really do anything at this point, we just skip them)

@birkjernstrom
Copy link
Member Author

No, was referring to one of the tasks in the issue, and the fact that I don't really know what it means. :)

Got it, now I'm following what you meant 😅

I've noticed at times that the progress bar can go to say 4/10 (as an example) issues synced and then suddenly start over again from 0/10.

This has happened during a proper installation vs. sync via the Github script. Thus not caught before during development since the script was always used to ease development of the feature.

It's possible it's no longer an issue as changes have been made. So unless you experience this, I think we can extract this as a separate issue (task) with low priority or even remove it entirely. It will resurface again as we build/polish if it remains an actual issue today.

I'd propose a third option of just updating the design to be "syncing x / y issues and PRs"?

Yeah, that's what I've settled at (even if the syncing for the PRs don't really do anything at this point, we just skip them)

Awesome 👍 Yeah, it's not "perfect", but it comes down to the strange dynamic of how Github treats PRs as issues and includes them in the count & paginated list. So given our reality, I think it's an acceptable compromise at least in an alpha.

@hult
Copy link
Contributor

hult commented Apr 12, 2023

#145 made it so any changes to a repo's open_issues is reflected in the syncing progress (because it uses the expected value from every stream event), and considers PRs "processed" (rather than synced) too. We don't send a stream event when skipping a PR (or if an error occurs), so the next time an issue is synced, the "processed" number will jump by more than one.

I'll close this issue, I think we can add another issue if we find the restarting happening again.

@hult hult closed this as completed Apr 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Archived in project
Development

No branches or pull requests

2 participants