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

Option to only write data back to disk when explicitly requested #123

Open
plannigan opened this issue Aug 9, 2021 · 3 comments
Open

Option to only write data back to disk when explicitly requested #123

plannigan opened this issue Aug 9, 2021 · 3 comments

Comments

@plannigan
Copy link

The parser currently will always write out the content to the file when a value is assigned to a property. This is not immediately obvious, as it is only hinted at by the docstring for fileobj.

The usage example doesn't mention anything about interacting with the file system, but will create or overwrite a file named Dockerfile file in the working directory.

If it would be to hard to adjust the library to work in a more explicit manor, it would be nice if the documentation were updated to be clear to someone finding the library for the first time about how it operates on files.

@tim-vk
Copy link
Contributor

tim-vk commented Oct 2, 2022

While I was adding some other stuff to this repo I'd thought to pick up some other issues. This one is quite strange for me:

I agree the usage example is a bit minimal. But the python code itself is actually quite nicely documented:

class DockerfileParser(object):
    def __init__(self, path=None,
                 cache_content=False,
                 env_replace=True,
                 parent_env=None,
                 fileobj=None,
                 build_args=None):
        """
        Initialize source of Dockerfile
        :param path: path to (directory with) Dockerfile; if not provided,
                     and fileobj is not provided, the current working
                     directory will be used
        :param cache_content: cache Dockerfile content inside DockerfileParser
        :param env_replace: return content with variables replaced
        :param parent_env: python dict of inherited env vars from parent image
        :param fileobj: seekable file-like object containing Dockerfile content
                        as bytes (will be truncated on write)
        :param build_args: python dict of build args used when building image
        """

The thing you are asking for (I think) is the cache_content input variable.
But there are two things that strike me as odd:

  1. why would you not want to edit the existing file? The only example I can think of is if you want to keep your original dockerfile and save an edited copy. But either way: you'd need two dockerfile parser instances (one to read, and one to edit/write). The cache_content variable seems useless to me.
  2. As far as I can see: the cache_content variable doesn't work. It was added 7 years ago: My guess is something broke in the meantime. But honestly: I think it is clutter that should be removed.

But I am curious if there are any use cases that I'm missing since it was added with a reason.

@Aisik00
Copy link

Aisik00 commented Jan 15, 2023

I am currently using this package for only loading the Dockerfile, to run static analysis on it (linting). So I was struggling a bit, that Dockerfile file is being created (rewritten) on file system even though I did not want that. If someone else is having similar problem, passing tempfile (tempfile.NamedTemporaryFile()) as fileobj argument helped.

@KevinMGranger
Copy link

I didn't know this was the case, and was surprised to find this out. I was already confused about why the library didn't just have a parse_file function that returns something that's just Plain Old Data.

Semi-related: I didn't realize the parsing was lazy-- I thought I could just create the DockerfileParser within the context manager, and then access its fields later.

Would y'all be opposed to me adding such an alternative API? It would be type-annotated and based on dataclasses. That requires 3.7, but 3.6 is EOL'd even by RHEL.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants