Use GitHub Actions Artifacts to store snapshots instead of S3

[StefMa]

Fixes #1676

I'm not sure if this is 100% correct.

AFAIK the new trigger events are:

• On each pull request (and updates on those)
• On push to master (probably on merge commits in this repo 🙃 )

I'm also not sure with the removal of the environment variables.
But I can't see where they are used, so I simply deleted them 🤷

I appreciate any help here, in case I did something wrong.

[IgnatBeresnev]

Hi! Thanks for giving it a shot!

A bit of context on how S3 upload and integration tests work. So integration tests are run and the result (Dokka output) is saved in a temporary folder (you don't know the path in advance, it has a somewhat random name). Then asserts are executed on it, and after that whatever was generated in that temporary folder gets copied to the folder specified in DOKKA_IT_AWS_PATH variable.

So I believe you still need this piece of configuration:

env:
    DOKKA_IT_AWS_PATH: /home/runner/work/dokka/stdlib

This should copy integration test results into the folder /home/runner/work/dokka/stdlib, and from there you can upload it as an artifact

[StefMa]

Hi,

thanks for the info @IgnatBeresnev .. Now it make sense.
Also when I look at

dokka/integration-tests/src/main/kotlin/org/jetbrains/dokka/it/S3Project.kt

Lines 10 to 14 in 50a3323

	fun copyToLocation() {
	System.getenv("DOKKA_IT_AWS_PATH")?.also { location ->
	println("Copying to ${File(location).absolutePath}")
	projectOutputLocation.copyRecursively(File(location))
	} ?: println("No copy path provided, skipping")

😁

We might also want to change this interface in this PR, right? 🤔
Probably something more generic like TestOutputLocationSaver...

[StefMa]

Another improvement we want/should make: compress the directory before we upload it to the artifacts.
Advantage: we would have a way smaller artifact size
Disadvantage: we end up downloading a zip in a zip

This is how github artifacts upload work. They upload "the artifacts like they are" but before downloading them, they zip it on the file. Therefore, if it's already zipped, they zip the zip on the server...

But I guess this is better than having artifact files of 186MBs 😳

This is how it look like before we download it

This is how it look like after we downloaded it

[IgnatBeresnev]

Hi again :)

We might also want to change this interface in this PR, right?

Yeah, making it something universal would be ideal! Both the interface and the environment variable.

---

Regarding 186M artifacts, I'm not sure what plan we're on and how much it's going to cost per month as it does look like quite a lot for every single commit. I've asked about it and I'll get back to you with an answer.

Anyhow, I think maybe it's not worth uploading stdlib output at all, serialization and coroutines usually are good examples and should suffice in the majority of cases.

There's no coroutines example project yet, but we have integration tests for it and it should be easy to add (preferably both for aws and action artifacts)

:integration-tests:gradle:integrationTest --tests org.jetbrains.dokka.it.gradle.kotlin.CoroutinesGradleIntegrationTest.kt --stacktrace

---

Speaking of AWS, unless there's an alternative with artifacts, I'm afraid it needs to stay. There are a number of use cases for it. Primarily it's used for demonstrating changes to non-technical colleagues or those not involved in Dokka's development. Giving them a link is certainly easier than sending everyone an archive, especially if it's a group discussion :)

I was hoping that maybe artifacts could be used in a similar way (maybe they would be automatically hosted on GH pages or available as files via direct paths), but I couldn't find anything like that :(

[IgnatBeresnev]

I think it's also worth adding a retention period of, say, 2 days for each artifact? This would save us some storage

Most likely it won't be needed as often, and I guess you would mostly want to look at the output in some short time after committing to verify your changes. If you forgot or didn't have enough time, you or the maintainers could re-trigger it

HowTo here

[StefMa]

Hi,

I changed the S3Project interface and restored the S3 GitHub Actions.

I also replaced the stdlib integration tests with the coroutines integration test (in the actions).
Should I delete the whole stdlib test? Or should it still be available (and run) but simply not uploaded to GH Artifacts and S3?

I also added the retation days. I think you're right and it shouldn't be available for that long (AFAIK 90 days by default).

But regarding pricing. AFAIK GitHub Actions are free for open source projects. According to this issue actions/upload-artifact#9 there is kinda of "no limit to upload".

Thanks for the feedback so far.

[IgnatBeresnev]

Nice, I think it's mostly ready! The only thing I'd ask is that stdlib be available in S3 projects as well. So it should be (in no particular order)

S3:

• stdlib
• coroutines
• serialization
• biojava

Artifacts:

• coroutines
• seialization
• biojava

But regarding pricing. AFAIK GitHub Actions are free for open source projects

Yeah, you're right. What confused me initially was that free plans still had storage/minute limits, but apparently it's for private repositories only :\ That being said, I think retention days can be increased to 5/7

---

Thanks for multiple rounds of review changes :)

[StefMa]

I did all the requested changes, but unfortunately I have no clue how to fix the current issue 😞
I didn't even get it run on my machine 😭

Can you help me here please?!

[Goooler]

@StefMa Try --no-daemon?

[StefMa]

@StefMa Try --no-daemon?

But is the coroutines documentation more heavy than the stdlib documentation? I doubt it. Because stdlib works out of the box, only coroutines has issues...

[Goooler]

But is the coroutines documentation more heavy than the stdlib documentation? I doubt it. Because stdlib works out of the box, only coroutines has issues...

Not sure, usually be flaky on Windows.

[StefMa]

Did I fixed it? 🤔

Maybe there was an bug in the previous used gradle version with the deamon which is fixed in this newer version... 🤷‍♂️

[IgnatBeresnev]

Looks like you have :) I was suspecting it was cache-redirector related (it's our internal proxy), and usually it helps to wait it out, but bumping the version is also OK

I've started integration tests, let's wait for it to complete and I think it's done! :)

[IgnatBeresnev]

LGTM, thank you so much for this (especially for many rounds of review), it'll help both maintainers and external contributors a whole lot!

[vmishenev]

Nice work!

[StefMa]

Thank you for the help too. I appreciate the feedback from you. It was really easy to get into the parts of the code where changes were required.