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

Automated Tagging enhancement #94

Merged
merged 2 commits into from
Mar 13, 2023
Merged

Conversation

rromannissen
Copy link
Contributor

@rromannissen rromannissen commented Feb 6, 2023

Signed-off-by: rromannissen rromannissen@gmail.com

One of the key assets in Konveyor is its tagging model that allows classifying applications in multiple dimensions. However, the task of tagging applications remains manual, which might not scale well when dealing with a large portfolio. The current enhancement aims at providing a way for addons to automatically tag applications based on the information they are able to surface. The starting point would be to leverage the technology stack information that Windup is able to collect during an analysis and translate that into tags that get automatically added to an application.

Fixes #95

Signed-off-by: rromannissen <rromannissen@gmail.com>
@shawn-hurley
Copy link
Contributor

shawn-hurley commented Feb 6, 2023

Should this be its own process and not explicitly tied to an analysis?

@jortel
Copy link
Contributor

jortel commented Feb 6, 2023

I think we'll want to add a option (checkbox) to the analysis wizard to indicate the user wants the analysis addon to tag the application. Default: true.

@rromannissen
Copy link
Contributor Author

I think we'll want to add a option (checkbox) to the analysis wizard to indicate the user wants the analysis addon to tag the application. Default: true.

@jortel that is already included under the "Changes in the Analysis Configuration Wizard" section from the functional specification: "The analysis configuration wizard should include an additional option to allow users to configure whether they want the analysis to automatically tag the included applications or not. This will be displayed as an additional switch in the advanced options view from the wizard. Mockups for this are WIP and will be added as soon as they are ready."

@rromannissen
Copy link
Contributor Author

Should this be its own process and not explicitly tied to an analysis?

@shawn-hurley that's the spirit of the enhancement: build something generic that any addon can leverage in the future, but deliver value on the first iteration with the current analysis addon AKA Windup.

@rromannissen
Copy link
Contributor Author

@PhilipCattanach we will need a list of all the tags available in Windup and their corresponding category. I know the list is available in the Red Hat downstream documentation here: https://access.redhat.com/documentation/en-us/migration_toolkit_for_applications/6.0/html/cli_guide/reference_material#tech-tags_cli-guide My guess is that if you were able to extract that, it shouldn't be hard to extract the categories as well, which would be required to seed Konveyor with that information as tag types(to be renamed to Categories as well) and tags.

Copy link
Contributor

@jortel jortel left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@shawn-hurley shawn-hurley left a comment

Choose a reason for hiding this comment

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

I would like to wait to merge this. I don't know the API that is being proposed from an analyzer perspective.

- Custom rules can identify additional tags, so the Windup addon should be able to create tags if they are not already present in the system.
- The source for tags coming from a Windup analysis will be "Analysis".
- If an analysis is run on an application that was analyzed previously, the list of tags associated to the "Analysis" source will be recreated unless the user disables the automated tagging in the advanced options screen from the analysis wizard.
- When running an analysis in the "Source + Dependencies" mode, the Windup CLI produces output for two differentiated applications, one being related to the source code of the application itself and the other to its dependencies. All technologies from both applications should be created as tags for the application that was analyzed. In other words, technologies coming from both the application source code and its dependencies should be taken into account and tagged accordingly.
Copy link
Contributor

Choose a reason for hiding this comment

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

These types of things need to be discussed from an API perspective with @djzager for the addons spec discussion IMO

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 don't think these belong to the addon API discussion, as they are more of a specification of how the Windup adapter layer on the addon should work. In other words, these are the specifics of how a given addon works, and don't define the API in any way. In fact, no changes to the API should be required to address this from what I understand, but I would ask @jortel to keep me honest here.

Copy link
Member

Choose a reason for hiding this comment

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

Now I'm confused. In the summary:

...the task of tagging applications remains manual... [this] enhancement aims at providing a way for addons to automatically tag applications...

Are we saying that this enhancement is specifically about Windup tagging applications? Or is this enhancement about automatically executing addons in general that tag applications?

If it's the latter, I think it's fair to suggest that the changes required to support that kind of change would be of interest to addons.

Copy link
Member

Choose a reason for hiding this comment

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

Offline it seems like it was the former, that this enhancement is specifically about Windup automatically tagging applications (not executing the windup addon or any other addon automatically). If @rromannissen can confirm this understanding, then I agree that there should be no addon API changes required.

My comment for this enhancement would then be to make the distinction more clear in the summary.

Copy link
Contributor

@jortel jortel Feb 7, 2023

Choose a reason for hiding this comment

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

Let's be careful with using the term auto or automatic here. The windup addon will simply parse the JSON output of Windup and use that information to ensure tags/categories exist and are associated with the application. This can be done using the existing APIs. The only additional thing is UI will need to pass an additional option (based on user input) in the Task.Data to the windup addon to indicate that it should tag (verb) the application.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, for the moment it's about having a way to represent tags coming from multiple sources and allowing addons to tag applications on demand. The automated execution of addons that tag applications can be discussed in future enhancements.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My use of automatic here comes from the fact that up until now, if users wants to tag an application, they have to do it manually through the UI. What this enhancement proposes is that, as a first step, after an analysis is completed some tags appear on the analyzed applications automatically, meaning with no human intervention aside from the configuration of the analysis parameters.

Signed-off-by: rromannissen <rromannissen@gmail.com>
@jortel jortel merged commit 46b0704 into konveyor:master Mar 13, 2023
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.

[RFE] Automated tagging of resources with Windup
4 participants