Background API #971

lbolla · 2021-08-26T11:15:22Z

Various improvements implemented in Molior fork.
See https://github.com/molior-dbs/aptly/tree/molior

Asynchronous execution of various API tasks to avoid locking the web interface (mirroring, publishing, snapshotting, etc.)
Batch updates to the database to avoid locking with transactions
Allow renaming repos
Improved logging for debugging
More robust retry strategy

lbolla · 2021-09-22T12:55:00Z

I ran "system" tests (the ones that used to run on TravisCI -- see #973 ) and fixed them to make them pass again (they were broken long time ago).

TESTS: 466 SUCCESS: 466 FAIL: 0 SKIP: 37

I'm attaching here the full logs for reference.
system-test-2021-09-22-12-36-29.log.gz

Note that I had to skip some tests which are using deprecated external systems or no-more-existing repositories. We'll have to update and fix those tests in another patch.

api/api.go

r4co0n · 2021-10-19T13:43:25Z

api/router.go

+	}
+
+	{
+		root.POST("/gpg/key", apiGPGAddKey)


Is a getter on /gpg/key not yet implemented?

Also, regarding the path name, I think this is confusing. We are adding public keys for mirror synchronization here, yet a central part of aptly is signing Release files using GPG (private) keys. Maybe /gpg/publickey would be a better name? Afaics, those public keys are used by aptly to:

Validate Release files downloaded on mirror update

Verify imported packages on repo include

The current path could be mistaken for the (not yet present) private key upload endpoint.

That's a fair point. @neolynx do you see problems with renaming? Probably Molior will have to adapt?

Ping @neolynx

r4co0n · 2021-10-19T13:45:17Z

api/gpg.go

+)
+
+// POST /api/gpg
+func apiGPGAddKey(c *gin.Context) {


~~I would love this function to also be able to just import a key I throw at it via HTTP. Has this just been not a need of yours, or where there problems implementing this functionality?~~

Please disregard this, I overlooked the GpgKeyArmor string and the --import part.

However, I am still unsure what would happen if I specify GpgKeyArmor and GpgKeyID attributes. Ideally, I could specify the KeyIDs to be imported from my ASC-File.

I would expect an error if any keys not found, ideally there would be an informational message about present keys that were not specified.

r4co0n · 2021-10-19T14:22:44Z

Relevant documentation would need to be updated separately so folks can use these changes, at least before the (desperately awaited) next release, or did I miss it? (PR is huge)

There should be another MR against aptly-dev/www.aptly.info. If you would like some help with that, I can try to prepare something not too wrong for you. ;)

Thanks for the good work, btw.

api/gpg.go

lbolla · 2021-10-27T13:42:31Z

Relevant documentation would need to be updated separately so folks can use these changes, at least before the (desperately awaited) next release, or did I miss it? (PR is huge)

There should be another MR against aptly-dev/www.aptly.info. If you would like some help with that, I can try to prepare something not too wrong for you. ;)

Thanks for the good work, btw.

Thanks! Definitely we need to document these changes properly. We are still in the process of getting credentials and operational knowledge from previous maintainers about how to do a release, how to update the website, etc. It's taking longer than we would have liked unfortunately.

If you have time for a documentation PR I would gladly accept it!

lbolla · 2021-10-27T13:45:11Z

Note to self: rebase this massive branch against master to fix conflicts.

lbolla · 2021-11-02T09:59:43Z

Just rebased this branch to master.

r4co0n · 2021-11-09T17:57:54Z

@lbolla, thanks for the additional information about what currently hinders a release, I was wondering. This is relevant for all the folks following #920.

r4co0n · 2021-11-09T18:02:24Z

If you have time for a documentation PR I would gladly accept it!

I definitely have time for this starting from the weekend of 2021-11-27, before that point I am uncertain, just so you know. :)

this is needed so concurrent reads and writes are possible.

This way db usage is safe.

Progress is not safe so for api its always nil and code needs to take care of this

… workflow.

PublishRepo26Test fails to run because something in the CI environment forces gpg to ask for the user's password. Try to require gpg1 for the test, which seems to run fine in other environments.

lbolla · 2022-01-26T08:30:05Z

@ximon18 As you can see, the CI is not running once I merged your GHA PR. Shall we maybe change the "branch" key in the on: pull_requests? Or just have on: pull_requests without further specification of the target branch?

ximon18 · 2022-01-26T08:38:12Z

@ximon18 As you can see, the CI is not running once I merged your GHA PR. Shall we maybe change the "branch" key in the on: pull_requests? Or just have on: pull_requests without further specification of the target branch?

Hi @lbolla, yes I mentioned this here.

It depends on the branching policy for development in this repository and when you want the checks to be run:

If development is sometimes or always done via direct pushes to main then on: pull_requests will either not be a good match or will not be insufficient.
If all development is done via pull requests from feature branches to main, or if you only care about catching issues just before they hit the main branch but not earlier, then you can specify the main branch as the target of the pull_requests event
If you use a two level deep branching strategy, e.g. all development is done via pull requests from feature branches into a dev branch then you would also need to specify the dev branch as a target of the pull_requests event.
If the number and depth of branches used is even deeper and/or varies and you always want changes to be tested then don't specify a target branch for the pull_requests event, the workflow will then run for all PRs.

ximon18 · 2022-01-26T08:42:05Z

Related to this discussion, on PR #1011 @randombenj just said:

I would strongly suggest going for on: push as not only main and PRs are tested but every single push so you immediately notice when something breaks ;)

The downside to this is when the PR is a work-in-progress and pushes are used as a sort of status update for collaborators and/or as a sort of backup, a lot of notifications can be annoying. It may also not make sense to keep running a workflow that takes a long time if not every result is going to be considered important/interesting, but instead only the final state of the branch prior to merge is interesting. I also don't know how many GH Actions credits this project has and if running very often will consume them.

See #1012

It may happen that aptly retries to download data during tests (maybe because of a network issue), but our fixtures doesn't account for it. So, we strip those irrelevant lines before comparison.

lbolla requested review from dario-gallucci, neolynx and szakalboss August 26, 2021 11:25

lbolla mentioned this pull request Sep 15, 2021

Is Aptly still maintained? #920

Closed

lbolla requested review from smira and sliverc September 20, 2021 10:51

lbolla mentioned this pull request Sep 22, 2021

Maintain backward compatibility in new task-based API #974

Closed

dario-gallucci approved these changes Oct 11, 2021

View reviewed changes

lbolla changed the title ~~Molior rebase~~ Background API Oct 11, 2021

r4co0n reviewed Oct 19, 2021

View reviewed changes

api/api.go Outdated Show resolved Hide resolved

r4co0n reviewed Oct 19, 2021

View reviewed changes

lebauce reviewed Oct 26, 2021

View reviewed changes

api/gpg.go Show resolved Hide resolved

lbolla force-pushed the molior-rebase branch from fa404b4 to 856649b Compare November 2, 2021 09:59

lbolla force-pushed the molior-rebase branch 2 times, most recently from 15c10ff to d23b78a Compare November 2, 2021 10:51

neolynx approved these changes Nov 2, 2021

View reviewed changes

lbolla force-pushed the molior-rebase branch from f519a12 to 326bc8c Compare December 16, 2021 13:13

Oliver Sauder added 5 commits January 26, 2022 09:13

every go routine needs to have its own collection factory

caf1cf1

this is needed so concurrent reads and writes are possible.

Removed obsolete RWMutexes

af48597

Database changes of resources need to be atomic

ae8910d

db batch may not be a global resource

483beef

This way db usage is safe.

Solving progress not safe issue for api

7476937

Progress is not safe so for api its always nil and code needs to take care of this

ximon18 and others added 11 commits January 26, 2022 09:14

golint: "Json" in func name should be "JSON".

6672ef4

govet: compose literal uses unkeyed fields

4023020

govet: compose literal uses unkeyed fields

356bc9f

Add a comment referring to the original Travis CI config from the new…

303c011

… workflow.

govet: compose literal uses unkeyed fields

63dc24c

govet: compose literal uses unkeyed fields

01627e3

Fix linting errors

04bf92a

Fix flake8 lint errors

adf978e

Fix some failing system tests

e2d2a2d

Try to fix test failing on CI

60cdcba

PublishRepo26Test fails to run because something in the CI environment forces gpg to ask for the user's password. Try to require gpg1 for the test, which seems to run fine in other environments.

Temporarily skip test failing on CI

4d62266

lbolla force-pushed the molior-rebase branch from 3ea6fa0 to 4d62266 Compare January 26, 2022 08:16

Fix test after merge

bd3dd23

randombenj and others added 11 commits January 26, 2022 13:58

Trigger CI on every push

27fac43

Build for newer go versions

613003b

Silence unhelpful linter error

1d51850

See #1012

Fix failing tests

c764a62

Re-enable testing on go 1.17

bd8507c

Ignore dates in test

dcb909d

Typo correction in GHA workflow comment

88c36d8

Strip irrelevant lines from test output

ca36bb5

It may happen that aptly retries to download data during tests (maybe because of a network issue), but our fixtures doesn't account for it. So, we strip those irrelevant lines before comparison.

Allow to check for empty output in tests

67dfa02

Update minimum required go version

110dba4

Only run system test with latest go version

1e91e29

lbolla merged commit a40cfc6 into master Jan 27, 2022

randombenj added this to the 1.5.0 milestone Jun 23, 2022

randombenj mentioned this pull request Nov 30, 2022

API: failures and race conditions with concurrent publish update operations #1125

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Background API #971

Background API #971

lbolla commented Aug 26, 2021

lbolla commented Sep 22, 2021 •

edited

Loading

r4co0n Oct 19, 2021

lbolla Oct 27, 2021

lbolla Dec 16, 2021

r4co0n Oct 19, 2021 •

edited

Loading

r4co0n commented Oct 19, 2021 •

edited

Loading

lbolla commented Oct 27, 2021

lbolla commented Oct 27, 2021

lbolla commented Nov 2, 2021

r4co0n commented Nov 9, 2021

r4co0n commented Nov 9, 2021

lbolla commented Jan 26, 2022

ximon18 commented Jan 26, 2022

ximon18 commented Jan 26, 2022 •

edited

Loading

Background API #971

Background API #971

Conversation

lbolla commented Aug 26, 2021

lbolla commented Sep 22, 2021 • edited Loading

r4co0n Oct 19, 2021

Choose a reason for hiding this comment

lbolla Oct 27, 2021

Choose a reason for hiding this comment

lbolla Dec 16, 2021

Choose a reason for hiding this comment

r4co0n Oct 19, 2021 • edited Loading

Choose a reason for hiding this comment

r4co0n commented Oct 19, 2021 • edited Loading

lbolla commented Oct 27, 2021

lbolla commented Oct 27, 2021

lbolla commented Nov 2, 2021

r4co0n commented Nov 9, 2021

r4co0n commented Nov 9, 2021

lbolla commented Jan 26, 2022

ximon18 commented Jan 26, 2022

ximon18 commented Jan 26, 2022 • edited Loading

lbolla commented Sep 22, 2021 •

edited

Loading

r4co0n Oct 19, 2021 •

edited

Loading

r4co0n commented Oct 19, 2021 •

edited

Loading

ximon18 commented Jan 26, 2022 •

edited

Loading