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

[JENKINS-59342] Builds / Nodes / Items bundles #198

Merged
merged 22 commits into from
Jan 14, 2020

Conversation

Dohbedoh
Copy link
Contributor

@Dohbedoh Dohbedoh commented Oct 18, 2019

JENKINS-59342: Provide the ability for Administrators to download contents for Builds, Items and Nodes. Details in the JIRA.

@MRamonLeon @aheritier @Evildethow @varyvol

@Dohbedoh Dohbedoh changed the title [JENKINS-59342] [JENKINS-59342] Builds / Nodes / Items bundles Oct 18, 2019
@Dohbedoh
Copy link
Contributor Author

Still need to check on those failing tests in windows.

Copy link

@varyvol varyvol left a comment

Choose a reason for hiding this comment

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

One question: what happens when the content to be zipped and downloaded is big? Are there issues creating the ZIP? Or downloading it?

* @param <C> Object that extends {@link AbstractModelObject}
* @return
*/
public <C extends AbstractModelObject> boolean isApplicable(Class<C> clazz) {
Copy link

Choose a reason for hiding this comment

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

Why did you need this now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To be able to have component that can be used at root level, and also at another level (Computer, Build, Item, ....). This method is used to list the components that can be used by from an Action. For example, we have a component DumpExportTable that get the exports table of all nodes when used from the root action. We can use the same component from the Node action to get the content only for the node.
Maybe there is a better design for this. I am open to suggestions.

@Dohbedoh Dohbedoh closed this Oct 24, 2019
@Dohbedoh Dohbedoh reopened this Oct 24, 2019
@Dohbedoh
Copy link
Contributor Author

One question: what happens when the content to be zipped and downloaded is big? Are there issues creating the ZIP? Or downloading it?

@varyvol This is going through the same method than the root Support Bundle generation. Subjected to the same technical vulnerabilities.

@Dohbedoh Dohbedoh closed this Oct 28, 2019
@Dohbedoh Dohbedoh reopened this Oct 28, 2019
@varyvol
Copy link

varyvol commented Dec 16, 2019

@Dohbedoh but the same technical limitations might work for a situation and not for others. For example, imagine a limitation is the bundle has a maximum capacity of 10mb that is very rarely reached with the usual bundle but will always be reached through this use case. That would make this use case unusable in the practice.

The code itself looks good, but there are test failures. Could you review them?

@Dohbedoh
Copy link
Contributor Author

Dohbedoh commented Dec 18, 2019

a limitation is the bundle has a maximum capacity of 10mb that is very rarely reached with the usual bundle but will always be reached through this use case.

I believe that if we were applying such kind of limitation, we could also add configurability to it, depending on what the scenario.


I will check on this. Tests are failing on Windows. I have tested the plugin in a instance running on windows and confirmed that it works. I haven't run the tests in a windows environment though...

@Dohbedoh
Copy link
Contributor Author

Dohbedoh commented Jan 2, 2020

I did find out that the ZIP entries in the case of windows have a mix of path separators, for example items/testFolder/jobs/testFreestyle/builds\1\build.xml. We should make sure that ZipEntry uses only forward slash.

@varyvol varyvol merged commit 4a3c124 into jenkinsci:master Jan 14, 2020
@Dohbedoh Dohbedoh deleted the JENKINS-59342 branch December 29, 2022 05:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants