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

Fields declaration order really matters when using multi-variables pattern #334

Open
devlounge opened this issue Jul 16, 2024 · 2 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@devlounge
Copy link

It looks to me that the order in which the dataclass fields are declared is super important.

Example directory structure:

environments
  dev
    config
      servers
        server-1
          server.yaml
        server-2
          server.yaml
        server-3
          server.yaml

Datafile definition:

@datafile("./environments/{self.environment}/config/servers/{self.name}/server.yaml")
class Server:
    name: str
    environment: str

When running:

import os

from manager.models import Cluster


if __name__ == '__main__':

    clusters = list(Server.objects.all())

It ends up raising:

FileNotFoundError: [Errno 2] No such file or directory: '/project/configuration/environments/server-1/config/servers/dev/server.yaml'

This seems to come from when the manager.all method does yield self.get(*values) with values ["server-1", "dev"] which ends up building the path posted above as manager.get iterates on the fields in the order they are declared and sets the values accordingly.

If I declare:

@datafile("./environments/{self.environment}/config/servers/{self.name}/server.yaml")
class Server:
    environment: str
    name: str

It works.

@devlounge devlounge changed the title Ordering Issue when trying to retrieve the file with a multi-variables pattern Fields declaration order really matters when using multi-variables pattern Jul 16, 2024
@jacebrowning
Copy link
Owner

jacebrowning commented Jul 17, 2024

Thanks for the clear steps to reproduce!

There may have been a reason it was implemented this way, but if someone wants to try to convert the parse() results into keyword arguments that may make this more robust:

result = parse(pattern, filename) or parse(alt_pattern, filename)
if result:
values = list(result.named.values())
if len(values) > 1 and os.sep in values[-1]:
parts = values[-1].rsplit(os.sep, 1)
values[-2] = values[-2] + os.sep + parts[0]
values[-1] = parts[1]
if _exclude and values[0].startswith(_exclude):
log.debug(f"Skipped loading of excluded value: {values[0]}")
continue
yield self.get(*values)

@jacebrowning jacebrowning added enhancement New feature or request help wanted Extra attention is needed labels Jul 17, 2024
@jacebrowning
Copy link
Owner

jacebrowning commented Jul 17, 2024

Alternatively, datafiles could raise an exception when the field ordering does not match the pattern.

Or we simply call out this gotcha in the documentation.

I'm open to suggestions!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants