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

Devfile 2.0 parser #2918

Closed
2 tasks done
elsony opened this issue Apr 17, 2020 · 13 comments
Closed
2 tasks done

Devfile 2.0 parser #2918

elsony opened this issue Apr 17, 2020 · 13 comments
Assignees
Labels
area/devfile-spec Issues or PRs related to the Devfile specification and how odo handles and interprets it. kind/user-story An issue of user-story kind triage/needs-information Indicates an issue needs more information in order to work on it.

Comments

@elsony
Copy link

elsony commented Apr 17, 2020

/kind user-story

User Story

Add devfile parser support to read devfiles that users the devfile 2.0 specification. Details of devfile 2.0 specification can be found under:
devfile/api#15

Devfile schema link: TBD

Acceptance Criteria

  • It should detect the version of devfile based on the apiVersion in devfile to decide if devfile 1.0 parser or devfile 2.0 parser is being used
  • It should generate the data model that represents devfile 2.0 model

Links

  • Related issue:

/kind user-story
/area devfile

@adisky
Copy link
Contributor

adisky commented Apr 23, 2020

To support Devfile 2.0 following things need to be done in odo.
Add folder 2.0.0 here,
https://github.com/openshift/odo/tree/master/pkg/devfile/parser/data
Add JSON schema for devfile 2.0.0
https://github.com/openshift/odo/tree/master/pkg/devfile/parser/data/2.0.0/devfileJsonSchema200.go
Define devfile 2.0.0 go struct Devfile200
https://github.com/openshift/odo/tree/master/pkg/devfile/parser/data/2.0.0/types.go
Implement the interface DevfileData with Devfile200 struct
https://github.com/openshift/odo/blob/master/pkg/devfile/parser/data/interface.go
https://github.com/openshift/odo/tree/master/pkg/devfile/parser/data/2.0.0/components.go
Add support for version 2.0.0 here
https://github.com/openshift/odo/blob/master/pkg/devfile/parser/data/versions.go

Depending upon devfile structure, we may need to modify the interface and add new methods.

@elsony @kadel If devfile 2.0 json schema is available, please share the link here.

@adisky
Copy link
Contributor

adisky commented Apr 23, 2020

@adisky
Copy link
Contributor

adisky commented Apr 23, 2020

@elsony
Copy link
Author

elsony commented May 5, 2020

@adisky Is there any outlook on the devfile 2.0 parser? There are quite a few items currently depending on that.

@adisky
Copy link
Contributor

adisky commented May 6, 2020

@elsony yes i am working on the parser.

few points i would like to get suggestions from team, problem here is Devfile v1 and v2 structures are completely different e.g.

// V1.0.0
type DevfileCommand struct {
	Actions []DevfileCommandAction 
	Attributes Attributes 
	Name string 
	PreviewUrl DevfileCommandPreviewUrl 
}
// V2.0.0
type DevfileCommand struct {
	Composite *Composite 
	Custom *Custom
	Exec *Exec 
	Type string 
	VscodeLaunch *VscodeLaunch 
	VscodeTask *VscodeTask 
}

It is very difficult to come to common types https://github.com/openshift/odo/blob/master/pkg/devfile/parser/data/common/types.go
This common types which is highly specific to v1 are being used everywhere in the adapters code e.g., we can say that adapters code is v1 specific.
https://github.com/openshift/odo/blob/master/pkg/devfile/adapters/common/command.go

I can think of following options to solve it.

  1. Common types modified to accomodate v2 types, (cons: maintenance of common structures, lots of checks in adapters about v1 and v2),
  2. Seperate code paths from adapters layer .(cons: maintenance of v1 and v2, pros: cleaner snd some duplicated code, v1 v2 check only at one place on CLI layer, removal of v1 is easy)
    https://github.com/openshift/odo/tree/master/pkg/devfile/adapters/v1
    https://github.com/openshift/odo/tree/master/pkg/devfile/adapters/v2

@adisky
Copy link
Contributor

adisky commented May 6, 2020

as discussed with @kadel, since it is difficult to map v2 types to common types, for now we are implementing the parser for v2 as separately.

@elsony
Copy link
Author

elsony commented May 6, 2020

I think it is fine to implement v2 parser separately. I guess the main question is more on how we can easily stage in the v2 functions. Before we have the functions based on v2 implemented enough to replace v1, we'll probably still need to keep the v1 functions around. To do that, the suggested adapter layer approach is probably the cleanest to implement.

@johnmcollier
Copy link
Member

@adisky Will your changes include implementing the v2 adapter?

@adisky
Copy link
Contributor

adisky commented May 6, 2020

@johnmcollier @elsony Since We were not sure about the adapter approach (creating adapter for each versions), I have added conversion structures from v1 to common, Please check the WIP PR
https://github.com/openshift/odo/pull/3106/files
With this current code should work with devfile v2 also. We can take two approaches from here to use the additional features of v2

  1. Modify the common structures, as needed for the new feature, and do not create separate adapter layer. use common structures in adapters
    https://github.com/openshift/odo/blob/master/pkg/devfile/parser/data/common/types.go
  2. Create layers for adapters package v1 and v2, and use direct structures of v1 and v2 in adapters package.

@elsony
Copy link
Author

elsony commented May 6, 2020

The current approach that we have is to covert the devfile v2 to the common structure which is currently based on v1.0. I think it is a simpler way to get the devfile v2 to work with existing code. In a long term, does it makes more sense to have the common structure based on the v2.0 and convert from v1.0 to the common which is based on v2.0? The reason that I am saying that is because by the time that we remove the experimental flag, we'll most likely remove the v1.0 support and only v2.0 devfile will be supported. So using v2.0 as the based for common structure may work better.

@adisky
Copy link
Contributor

adisky commented May 7, 2020

The current approach that we have is to covert the devfile v2 to the common structure which is currently based on v1.0. I think it is a simpler way to get the devfile v2 to work with existing code. In a long term, does it makes more sense to have the common structure based on the v2.0 and convert from v1.0 to the common which is based on v2.0? The reason that I am saying that is because by the time that we remove the experimental flag, we'll most likely remove the v1.0 support and only v2.0 devfile will be supported. So using v2.0 as the based for common structure may work better.

yes in the long run, common structures would be based on V2.0. We can modify the common structures one by one, so that it does not break adapters, and also with the features addition based on v2.0.

@elsony
Copy link
Author

elsony commented May 7, 2020

This approach sounds good to me

@adisky
Copy link
Contributor

adisky commented Jun 3, 2020

closed by #3216

@adisky adisky closed this as completed Jun 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/devfile-spec Issues or PRs related to the Devfile specification and how odo handles and interprets it. kind/user-story An issue of user-story kind triage/needs-information Indicates an issue needs more information in order to work on it.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants