Skip to content
This repository has been archived by the owner on Oct 10, 2023. It is now read-only.

adding proposal for dockerfile components #242

Closed
wants to merge 1 commit into from

Conversation

mike-hoang
Copy link
Contributor

@mike-hoang mike-hoang commented Jun 19, 2023

What does this PR do?

Initial draft for improving port detection logic for cases where the dockerfiles are not found in a component.

Which issue(s) does this PR fix

fixes #244

PR acceptance criteria

Testing and documentation do not need to be complete in order for this PR to be approved. We just need to ensure tracking issues are opened.

  • Unit/Functional tests

  • Documentation

How to test changes / Special notes to the reviewer


## Potential Solution

Introduce a new component field called `orphan_component` and add it to the `Dockerfile` language:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do we call it orphan_component instead of Dockerfile component?

Copy link
Collaborator

Choose a reason for hiding this comment

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

In the example above, what is the priority of the search order? If there is a Dockerfile at the root, do we consider that as a Dockerfile component?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We decided not to use "docker_component" or anything that is closely coupled to Dockerfiles so we can resuse the field if there were ever another "orphaned" component (as in we wanted to define a component like Dockerfile where there isn't really a language or framework tied to it.)

We also didn't want to use the existing field component as it doesn't really match the others (i.e. C#, F#, Go, Java, etc) because they're languages, compared to a single Dockerfile.

Also open to renaming the field too if there's something more accurate

Copy link
Contributor Author

@mike-hoang mike-hoang Jun 21, 2023

Choose a reason for hiding this comment

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

In the example above, the output would look something like this:

[
  {
    // component found; since there is a dockerfile inside, it will use the Dockerfile strategy for port detection
    "Name": ...,
    "Path": "repo-being-detected-by-alizer/component/",
    ...
  },
  {
    // the orphaned component where just the Dockerfile is used
    "Name": ...,
    "Path": "repo-being-detected-by-alizer/"
  }
]

As to the priority of the search, I think currently, the order would be random / depends on the list of component paths found. Regardless of order, it will go through both components as reflected in the output.

The proposed changes will consider the single Dockerfile in the root as an "orphaned" component

Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you mean Dockerfile support will be handled differently in those two cases? Do we still try to do port detection for Dockerfile on root? It is still not clear to me why those two dockerfile cases will need to be processed differently.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

+1 for nonlanguage_component

Copy link
Collaborator

@kim-tsao kim-tsao Jun 29, 2023

Choose a reason for hiding this comment

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

or container_component? Since Dockerfile is part of language.yaml, Linguist considers it a language. To call it a nonlanguage would be confusing in this context. I know you mentioned you wanted to make this generic enough to future proof this but if we ever do encounter some other use case, maybe we can just add a new component classification and treat it in the same way as container_component?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is a good point, I can't really think of another use case and creating a new component classification wouldn't be hard since each enricher is language specific

If we did want to stick with a generic name, maybe we could use domain_specific_component? Docker does consider the Dockerfile syntax to be a DSL

Copy link
Collaborator

Choose a reason for hiding this comment

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

As long as we are confident enough that DSLs are always going to be processed this way: https://en.wikipedia.org/wiki/Domain-specific_language

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Settled on container_component with the intent that new component classifications can be created in the future if needed

Signed-off-by: Michael Hoang <mhoang@redhat.com>
@mike-hoang mike-hoang self-assigned this Jun 27, 2023
@mike-hoang mike-hoang changed the title wip: adding proposal for dockerfile components adding proposal for dockerfile components Jun 27, 2023
@mike-hoang mike-hoang marked this pull request as ready for review June 27, 2023 13:47
@mike-hoang mike-hoang requested a review from elsony June 28, 2023 13:32
@mike-hoang
Copy link
Contributor Author

PR moved to devfile/alizer#3

@mike-hoang mike-hoang closed this Jun 30, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create proposal for defining a dockerfile component
3 participants