Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

New Free license adjustments#46504

Merged
pjlast merged 11 commits intomainfrom
pjlast/45640-free-licensing-updates
Feb 17, 2023
Merged

New Free license adjustments#46504
pjlast merged 11 commits intomainfrom
pjlast/45640-free-licensing-updates

Conversation

@pjlast
Copy link
Contributor

@pjlast pjlast commented Jan 16, 2023

Closes #45640

Only intended to be merged once we want the new free license to go live.

Test plan

Added unit tests where appropriate. I did manual tests as well.

@cla-bot cla-bot bot added the cla-signed label Jan 16, 2023
@pjlast pjlast requested a review from a team January 17, 2023 12:42
@pjlast pjlast force-pushed the pjlast/45640-free-licensing-updates branch from 20c3554 to 545b3bf Compare January 30, 2023 13:03
@pjlast pjlast force-pushed the pjlast/45640-free-licensing-updates branch from d30ca74 to 2c821ad Compare February 16, 2023 07:29
@pjlast pjlast marked this pull request as ready for review February 16, 2023 07:45
@github-actions
Copy link
Contributor

Problem: the label i-acknowledge-this-goes-into-the-release is absent.
👉 What to do: we're in the next Sourcegraph release code freeze period. If you are 100% sure your changes should get released or provide no risk to the release, add the label your PR with i-acknowledge-this-goes-into-the-release.

@sourcegraph-bot
Copy link
Contributor

sourcegraph-bot commented Feb 16, 2023

Codenotify: Notifying subscribers in CODENOTIFY files for diff 50c9fb6...69c5421.

Notify File(s)
@indradhanush enterprise/cmd/repo-updater/shared/licensing.go
enterprise/cmd/repo-updater/shared/licensing_test.go
enterprise/cmd/repo-updater/shared/shared.go
internal/repos/syncer.go
@sashaostrikov internal/repos/syncer.go
@unknwon enterprise/cmd/frontend/internal/licensing/enforcement/users.go
enterprise/internal/licensing/data.go
enterprise/internal/licensing/features.go
enterprise/internal/licensing/licensing.go
enterprise/internal/licensing/plans.go

return nil
}

numPrivateRepos, err := s.RepoStore().Count(ctx, ossDB.ReposListOptions{OnlyPrivate: true})
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't know if this is true:

What about we do the SQL query like:

SELECT COUNT(*) >= $1 FROM repo WHERE private

In the hope that the PostgreSQL is smart enough to stop counting once it for sure knows the return value will be TRUE?


The benefit of doing it this way:

  1. We would not be so concerned about a large number of public repos in the table slowing down the query too much.
  2. Kinda doing better at avoiding race conditions like creating tons of repos in a short amount of time.

Maybe this is not worth it at all, given the assumption that repo-number-restricted instance would only have a small number of repos after all. 😅

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmmm might be worth it 🤔 Also not sure if Postgres would optimise this in some way. Let me try and find out

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 can't find evidence that this would be made any faster. But yes, there should not be a large amount of private repositories, and the only time this could be slow if someone is changing from a license where private repos is allowed, and they have a large amount of private repositories, to a license where it is restricted. And then only in the transitioning period could it maybe be slow

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed, I don't think it is worth optimizing the query too much, since it's OK to penalize people with lots of private repos who exceed the license.

@pjlast pjlast requested a review from a team February 16, 2023 12:38
return nil
}

numPrivateRepos, err := s.RepoStore().Count(ctx, ossDB.ReposListOptions{OnlyPrivate: true})
Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed, I don't think it is worth optimizing the query too much, since it's OK to penalize people with lots of private repos who exceed the license.

@pjlast pjlast merged commit 3337387 into main Feb 17, 2023
@pjlast pjlast deleted the pjlast/45640-free-licensing-updates branch February 17, 2023 07:00
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Implement updates to free-licensing tier (final step of 4.0 P&P updates)

5 participants