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

Define document nodes content with template #64

Merged
merged 2 commits into from
Oct 26, 2020
Merged

Conversation

g-pavlov
Copy link
Contributor

@g-pavlov g-pavlov commented Oct 25, 2020

What this PR does / why we need it:
Adds the third option for content assignment to document nodes - templates.
Contributes file system resource handler enabling local file system nodes in structures and git info for them.
With local file system support, template paths can be local file system paths too.

Which issue(s) this PR fixes:
Fixes #18
Completes #54

Release note:

Document nodes now support content assignment with Template.
Document nodes now support content assignment from local file system sources. NodeSelectors with local file path are supported but the optional filters of the selectors are not implemented yet.
Git info is supported for document nodes with local file system content assignment. The author/contributor details are limited to  what `git log` can offer - email and name. Requires `git` to be available and sufficient privileges for `docforge` to run `git log`.

@g-pavlov g-pavlov added area/documentation Documentation related kind/enhancement Enhancement, improvement, extension labels Oct 25, 2020
@g-pavlov g-pavlov self-assigned this Oct 25, 2020
@gardener-robot gardener-robot added kind/api-change API change with impact on API users needs/second-opinion Needs second review by someone else needs/review Needs review size/l Size of pull request is large (see gardener-robot robot/bots/size.py) labels Oct 25, 2020
@gardener-robot-ci-3 gardener-robot-ci-3 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Oct 25, 2020
@gardener-robot-ci-2 gardener-robot-ci-2 added needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Oct 25, 2020
@gardener-robot-ci-2 gardener-robot-ci-2 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Oct 25, 2020
@gardener-robot-ci-1 gardener-robot-ci-1 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Oct 25, 2020
@gardener-robot-ci-1 gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Oct 25, 2020
@gardener-robot-ci-3 gardener-robot-ci-3 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Oct 25, 2020
@gardener-robot-ci-2 gardener-robot-ci-2 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Oct 25, 2020
@gardener-robot-ci-3 gardener-robot-ci-3 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Oct 25, 2020
@gardener-robot-ci-2 gardener-robot-ci-2 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Oct 25, 2020
@gardener-robot-ci-3 gardener-robot-ci-3 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Oct 25, 2020
@gardener-robot-ci-2 gardener-robot-ci-2 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Oct 25, 2020
pkg/api/types.go Outdated
@@ -234,7 +234,7 @@ type Template struct {
// Sources maps variable names to ContentSelectors that will be
// used as specification for the content to fetch and assign ot that
Copy link
Contributor

Choose a reason for hiding this comment

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

used as specification for the content to fetch and assign to those variables

templateBlob []byte
tmpl *template.Template
)
if tmpl, ok = w.templates[task.Node.Template.Path]; !ok {
Copy link
Contributor

Choose a reason for hiding this comment

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

could we have concurrent reads


// Accept implements resourcehandlers.ResourceHandler#Accept
func (fs *fsHandler) Accept(uri string) bool {
if _, err := os.Stat(uri); err == nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

we could just return err == nil

if path != parentPath {
if len(strings.Split(path, "/"))-len(strings.Split(parentPath, "/")) != 1 {
node = node.Parent()
parentPathSegments := strings.Split(path, "/")
Copy link
Contributor

Choose a reason for hiding this comment

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

pathSegments intead of parentPathSegments

return nil
}

func buildNodes(path string, info os.FileInfo) *api.Node {
Copy link
Contributor

Choose a reason for hiding this comment

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

is this used

}

git := exec.Command("git", "log", "--date=short", `--pretty=format:'{%n "sha": "%H",%n "author": "%aN <%aE>",%n "date": "%ad",%n "message": "%s",%n "email": "%aE",%n "name": "%aN"%n },'`, "--follow", path)
if log, err = git.CombinedOutput(); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think if we use CombinedOutput here and the git command fails the stderr will be again saved to log. Is this desired?

@gardener-robot gardener-robot added needs/changes Needs (more) changes and removed needs/review Needs review labels Oct 26, 2020
@gardener-robot-ci-1 gardener-robot-ci-1 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Oct 26, 2020
@g-pavlov
Copy link
Contributor Author

@swilen-iwanow ready for review after fixing requested changes

@g-pavlov g-pavlov added needs/review Needs review and removed needs/second-opinion Needs second review by someone else labels Oct 26, 2020
@gardener-robot gardener-robot removed the needs/review Needs review label Oct 26, 2020
@gardener-robot gardener-robot added the needs/second-opinion Needs second review by someone else label Oct 26, 2020
@gardener-robot-ci-2 gardener-robot-ci-2 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Oct 26, 2020
@gardener-robot-ci-3 gardener-robot-ci-3 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Oct 26, 2020
@gardener-robot-ci-1 gardener-robot-ci-1 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Oct 26, 2020
@swilen-iwanow
Copy link
Contributor

lgtm

@swilen-iwanow swilen-iwanow merged commit 100dd66 into master Oct 26, 2020
@g-pavlov g-pavlov deleted the template branch October 26, 2020 16:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/documentation Documentation related kind/api-change API change with impact on API users kind/enhancement Enhancement, improvement, extension needs/changes Needs (more) changes needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) needs/second-opinion Needs second review by someone else size/l Size of pull request is large (see gardener-robot robot/bots/size.py)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

File system resource handler
6 participants