Skip to content

Default template dir#176

Closed
jmeixensperger wants to merge 2 commits intodevelopfrom
bug/template-dir
Closed

Default template dir#176
jmeixensperger wants to merge 2 commits intodevelopfrom
bug/template-dir

Conversation

@jmeixensperger
Copy link
Copy Markdown
Contributor

@jmeixensperger jmeixensperger commented Apr 22, 2019

Use sampleDir to define a default templates path and load it into our filesystem.
Addresses #167.

GordonWang
GordonWang reviewed Apr 23, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@GordonWang GordonWang left a comment

Choose a reason for hiding this comment

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

Can we hold this change a while. and let's continue discussion when I create my PR

splunk_eventgen/lib/plugins/generator/jinja.py
working_dir = os.path.join(splunk_home, 'etc', 'apps', app_name, 'default')

default_template_dir = os.path.join(os.path.dirname(working_dir), 'samples', 'templates')
if hasattr(self._sample, "sampleDir"):
Copy link
Copy Markdown
Contributor

@GordonWang GordonWang Apr 23, 2019

Choose a reason for hiding this comment

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

I have fixed #176 in my local branch and will submit the PR soon.

I think this only fixed part of the issue. for #176, we need to fix 2 parts

  1. resolve the jinja_template_dir with sampleDir when it is a relative path
  2. use the jinja_template_dir directly when it is a absolute path.

Copy link
Copy Markdown
Contributor

@GordonWang GordonWang Apr 23, 2019

Choose a reason for hiding this comment

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

This issue is addressed in #177

Please review this.

Copy link
Copy Markdown
Contributor

@li-wu li-wu Apr 24, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

@jmeixensperger could we just use #177 since it addressed both the default template dir issue and vulnerability issue with test cases covered?

li-wu
li-wu reviewed Apr 24, 2019
View reviewed changes
setup.py
'jinja2',
'pyrabbit==1.1.0',
'urllib3==1.23',
'urllib3>=1.24.2',
Copy link
Copy Markdown
Contributor

@li-wu li-wu Apr 24, 2019

Choose a reason for hiding this comment

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

Also this should be urllib3==1.24.2, or it will break all the functional tests.

@GordonWang
Copy link
Copy Markdown
Contributor

GordonWang commented Apr 24, 2019

#177 has been merged.

@arctan5x
Copy link
Copy Markdown
Contributor

arctan5x commented Apr 25, 2019

Please resolve merge conflicts.

@li-wu
Copy link
Copy Markdown
Contributor

li-wu commented Apr 26, 2019

@arctan5x @jmeixensperger I think both the default template dir issue and the vulnerability issue are already fixed at #177 with test cases. So this PR can be closed instead of merging to dev branch.

@li-wu li-wu deleted the bug/template-dir branch August 13, 2019 22:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

4 more reviewers

@li-wu li-wu li-wu approved these changes

@GordonWang GordonWang GordonWang left review comments

@lephino lephino lephino approved these changes

@arctan5x arctan5x arctan5x approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@jmeixensperger @GordonWang @arctan5x @li-wu @lephino