[JIRA] (JENKINS-55829) Expose AbstractCIBase#disabledAdministrativeMonitors as internal API

7 views
Skip to first unread message

o.v.nenashev@gmail.com (JIRA)

unread,
Jan 29, 2019, 3:50:02 AM1/29/19
to jenkinsc...@googlegroups.com
Oleg Nenashev created an issue
 
Jenkins / Improvement JENKINS-55829
Expose AbstractCIBase#disabledAdministrativeMonitors as internal API
Issue Type: Improvement Improvement
Assignee: Unassigned
Components: core
Created: 2019-01-29 08:49
Labels: newbie-friendly
Priority: Minor Minor
Reporter: Oleg Nenashev

This is a follow-up to https://github.com/jenkinsci/jenkins/pull/3240 , see these comments:

It would be nice to create a new getter method in AbstractCIBase and to mark it as @Restricted(NoExternalUse.class) so that it becomes a core-internal API. It would help to avoid weird-looking and potentially unsafe package-level casts

 

 

Add Comment Add Comment
 
This message was sent by Atlassian Jira (v7.11.2#711002-sha1:fdc329d)

nisshah1499@gmail.com (JIRA)

unread,
Jan 29, 2019, 11:38:01 AM1/29/19
to jenkinsc...@googlegroups.com
Nisarg Shah commented on Improvement JENKINS-55829
 
Re: Expose AbstractCIBase#disabledAdministrativeMonitors as internal API

I have a doubt that getter method in AbstractCIBase should be created which assigns all the values of parameters from Jenkins.get() right?

o.v.nenashev@gmail.com (JIRA)

unread,
Jan 29, 2019, 11:42:01 AM1/29/19
to jenkinsc...@googlegroups.com

I do not get the question, sorry. This issue is just about adding one Getter method for a single field

nisshah1499@gmail.com (JIRA)

unread,
Jan 29, 2019, 11:58:05 AM1/29/19
to jenkinsc...@googlegroups.com

Okay so just instance of jenkins from Jenkins.get() needs to be stored by that method. Am I correct?

nisshah1499@gmail.com (JIRA)

unread,
Jan 31, 2019, 8:37:02 AM1/31/19
to jenkinsc...@googlegroups.com
Nisarg Shah updated an issue
 
Change By: Nisarg Shah
Comment:
Okay so just instance of jenkins from Jenkins.get() needs to be stored by that method. Am I correct?

nisshah1499@gmail.com (JIRA)

unread,
Jan 31, 2019, 9:02:02 AM1/31/19
to jenkinsc...@googlegroups.com
 
Re: Expose AbstractCIBase#disabledAdministrativeMonitors as internal API

I would like to do this task. Can i assign it to me?

nisshah1499@gmail.com (JIRA)

unread,
Jan 31, 2019, 9:02:02 AM1/31/19
to jenkinsc...@googlegroups.com
Nisarg Shah edited a comment on Improvement JENKINS-55829
I would like to do this task. Can i I assign it to me?

o.v.nenashev@gmail.com (JIRA)

unread,
Jan 31, 2019, 9:16:02 AM1/31/19
to jenkinsc...@googlegroups.com

Yes, thanks. If a task is not assigned, just feel free to take it. No need to ask

nisshah1499@gmail.com (JIRA)

unread,
Jan 31, 2019, 9:55:02 AM1/31/19
to jenkinsc...@googlegroups.com

nisshah1499@gmail.com (JIRA)

unread,
Jan 31, 2019, 9:55:02 AM1/31/19
to jenkinsc...@googlegroups.com

nisshah1499@gmail.com (JIRA)

unread,
Jan 31, 2019, 10:27:02 AM1/31/19
to jenkinsc...@googlegroups.com

Oleg Nenashev Sorry for the inconvenience occurred in the pull request. There were some issues due to which it happened, so I just closed previous pull request and now I will do two individual PR.

nisshah1499@gmail.com (JIRA)

unread,
Jan 31, 2019, 1:05:03 PM1/31/19
to jenkinsc...@googlegroups.com

nisshah1499@gmail.com (JIRA)

unread,
Jan 31, 2019, 10:20:02 PM1/31/19
to jenkinsc...@googlegroups.com

I think error is due to the “Restricted(NoExternalUse.class)” as shown in the tests. And please can you help me that how can I compile and run the complete code on my local computer and can do all these tests on local?

nisshah1499@gmail.com (JIRA)

unread,
Feb 1, 2019, 1:44:01 PM2/1/19
to jenkinsc...@googlegroups.com

Can you please provide me some guidance on how to compile and run this code on local and do these tests. As in test, error of "Restricted" is shown. So I can just check it on my local.

And do you think that changes done(new getter method) are perfect or I need to work on that?

o.v.nenashev@gmail.com (JIRA)

unread,
Feb 1, 2019, 2:03:01 PM2/1/19
to jenkinsc...@googlegroups.com

nisshah1499@gmail.com (JIRA)

unread,
Feb 1, 2019, 2:16:01 PM2/1/19
to jenkinsc...@googlegroups.com

Oleg Nenashev Thank you. I will just look into it.

And any suggestions in PR?

nisshah1499@gmail.com (JIRA)

unread,
Feb 5, 2019, 12:00:02 PM2/5/19
to jenkinsc...@googlegroups.com

Oleg Nenashev

 

I have seen your suggestions and also applied on the code. But then, when I tried to compile it on my laptop than same compilation error is observed.

Discussions :

https://github.com/jenkinsci/jenkins/pull/3873#issuecomment-460027715

https://github.com/jenkinsci/jenkins/pull/3873#discussion_r252975695

 

Any suggestions ?

nisshah1499@gmail.com (JIRA)

unread,
Feb 6, 2019, 12:28:02 PM2/6/19
to jenkinsc...@googlegroups.com

Oleg Nenashev

The compilation error is due to the tag @Restricted(NoExternalUse.class). So any suggestions what should be done to remove this compilation error?

o.v.nenashev@gmail.com (JIRA)

unread,
Feb 6, 2019, 12:37:01 PM2/6/19
to jenkinsc...@googlegroups.com

import classes properly?

Have you been able to open Jenkins as a project in IDE?

nisshah1499@gmail.com (JIRA)

unread,
Feb 6, 2019, 12:55:02 PM2/6/19
to jenkinsc...@googlegroups.com

I have tried to run command of mvn on Jenkins folder after the fix. And in that, compilation error is shown.

Yes I have tried to open Jenkins as a project in IDE but not tried to test Jenkins fix itself in IDE.

nisshah1499@gmail.com (JIRA)

unread,
Feb 6, 2019, 1:01:02 PM2/6/19
to jenkinsc...@googlegroups.com

nisshah1499@gmail.com (JIRA)

unread,
Feb 6, 2019, 1:02:01 PM2/6/19
to jenkinsc...@googlegroups.com

nisshah1499@gmail.com (JIRA)

unread,
Feb 8, 2019, 11:31:02 PM2/8/19
to jenkinsc...@googlegroups.com

The compilation error is due to the tag @Restricted(NoExternalUse.class). So any suggestions what should be done to remove this compilation error?

 

nisshah1499@gmail.com (JIRA)

unread,
Feb 9, 2019, 1:16:02 AM2/9/19
to jenkinsc...@googlegroups.com
Nisarg Shah edited a comment on Improvement JENKINS-55829
[~oleg_nenashev]

The I have committed suggested changes but the compilation error continues which is due to the tag @Restricted(NoExternalUse.class). So any suggestions what should be done to remove this compilation error?

 

nisshah1499@gmail.com (JIRA)

unread,
Feb 10, 2019, 9:33:02 AM2/10/19
to jenkinsc...@googlegroups.com

Can we just remove @Restricted(NoExternalUse.class) and try?

nisshah1499@gmail.com (JIRA)

unread,
Feb 10, 2019, 9:34:02 AM2/10/19
to jenkinsc...@googlegroups.com
Nisarg Shah edited a comment on Improvement JENKINS-55829
Can If possible can we just remove @Restricted(NoExternalUse.class) and try? I know it's not the correct way but if possible.

o.v.nenashev@gmail.com (JIRA)

unread,
Feb 10, 2019, 2:39:02 PM2/10/19
to jenkinsc...@googlegroups.com

You just need to import the classes

 

import org.kohsuke.accmod.Restricted;

import org.kohsuke.accmod.restrictions.NoExternalUse;

 

nisshah1499@gmail.com (JIRA)

unread,
Feb 11, 2019, 12:02:01 PM2/11/19
to jenkinsc...@googlegroups.com

nisshah1499@gmail.com (JIRA)

unread,
Feb 11, 2019, 12:03:02 PM2/11/19
to jenkinsc...@googlegroups.com
Nisarg Shah updated an issue
Change By: Nisarg Shah
Comment:
If possible can we just remove @Restricted(NoExternalUse.class) and try? I know it's not the correct way but if possible.

o.v.nenashev@gmail.com (JIRA)

unread,
Feb 11, 2019, 12:07:02 PM2/11/19
to jenkinsc...@googlegroups.com
Oleg Nenashev commented on Improvement JENKINS-55829
 
Re: Expose AbstractCIBase#disabledAdministrativeMonitors as internal API

It is because you replaced the field in https://github.com/jenkinsci/jenkins/pull/3873/commits/dbebe2152707e67548cd3ad74c0955724d486c7b . Now you just return new empty container in each call, and hence all tests depending on the data persistency are just failing

nisshah1499@gmail.com (JIRA)

unread,
Feb 11, 2019, 12:54:01 PM2/11/19
to jenkinsc...@googlegroups.com

Sorry I didn't get it. Can you explain it again?

nisshah1499@gmail.com (JIRA)

unread,
Feb 14, 2019, 12:03:01 PM2/14/19
to jenkinsc...@googlegroups.com

Oleg Nenashev

So do I need to return extra new empty container from the getter method? Is this what you are suggesting?

nisshah1499@gmail.com (JIRA)

unread,
Feb 24, 2019, 8:40:02 AM2/24/19
to jenkinsc...@googlegroups.com
Nisarg Shah updated an issue
Change By: Nisarg Shah
Comment: [~oleg_nenashev]


So do I need to return extra new empty container from the getter method? Is this what you are suggesting?

nisshah1499@gmail.com (JIRA)

unread,
Mar 13, 2019, 12:09:02 PM3/13/19
to jenkinsc...@googlegroups.com
 
Re: Expose AbstractCIBase#disabledAdministrativeMonitors as internal API

Oleg Nenashev

Can you please review the PR : https://github.com/jenkinsci/jenkins/pull/3873

Changes have been made according to the suggestions and now it looks fine !!

 

nisshah1499@gmail.com (JIRA)

unread,
May 9, 2019, 7:17:02 AM5/9/19
to jenkinsc...@googlegroups.com

Oleg Nenashev As this issue is solved, I request you to make resolve label on this issue so that no other contributor pick this issue.

o.v.nenashev@gmail.com (JIRA)

unread,
May 9, 2019, 7:19:01 AM5/9/19
to jenkinsc...@googlegroups.com
Oleg Nenashev resolved as Fixed
 

Right, I forgot to do that. 

Change By: Oleg Nenashev
Status: Open Resolved
Resolution: Fixed
Reply all
Reply to author
Forward
0 new messages