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

query with alternative records dependency on different alt #1092

Open
relikd opened this issue Nov 23, 2022 · 9 comments · May be fixed by #1093
Open

query with alternative records dependency on different alt #1092

relikd opened this issue Nov 23, 2022 · 9 comments · May be fixed by #1093

Comments

@relikd
Copy link
Contributor

relikd commented Nov 23, 2022

Lets say, I use site.query('/path', this.alt)|list in a template. This will not only include the records of the provided alt, but also the records for the default alt.

Assuming the default alt is EN:

  • layout-en.html with query('/pages', alt='en') tracks dependency on page1-en.html and page2-en.html.
  • layout-de.html with query('/pages', alt='de') tracks dependency on page1-en.html, page2-en.html, page1-de.html, and page2-de.html.
@relikd
Copy link
Contributor Author

relikd commented Nov 23, 2022

I assume it is because to_fs_path ignores the alt?

ctx.record_dependency(self.pad.db.to_fs_path(self.path))

EDIT: probably not. At least doesnt change the dependencies if I comment out both track_record_dependency and record_dependency.

@relikd
Copy link
Contributor Author

relikd commented Nov 23, 2022

This is my SQL query to check what dependencies have been recorded after a clean build. The count does not change if I create the query, but it changes as soon as I iteratate over the entries, or if I get the parents children (site.get('/path').children|list).

select artifact,source from artifacts where artifact like "de/%" and source like "%+en.lr";

@relikd relikd linked a pull request Nov 23, 2022 that will close this issue
2 tasks
@relikd
Copy link
Contributor Author

relikd commented Nov 23, 2022

This should fix it. But what about this? This is missing the alt too. However, I cannot assess how it affects attachements.

lektor/lektor/db.py

Lines 762 to 767 in fcf31cb

@property
def parent(self):
"""The associated record for this attachment."""
return self.pad.get(
self._data["_attachment_for"], persist=self.pad.cache.is_persistent(self)
)

@relikd
Copy link
Contributor Author

relikd commented Nov 24, 2022

@dairiki ok, my last comment needs an alt too to fix attachment queries. E.g., record.attachments.images creates a query which seems to access the parent of each attachment. I dont know why this is needed but this effectively drops the alt and records a dependency on a foreign alt.

@dairiki
Copy link
Contributor

dairiki commented Nov 24, 2022

Attachments do have an alt attribute, I think. (It only affects the attributes stored in associated .lr. There is only one alt-independent URL per attachment.)

@relikd
Copy link
Contributor Author

relikd commented Nov 24, 2022

That was not the question. Yes, Attachments do have an alt attribute, as you said, for .lr. But accessing the attachments will record a wrong dependency:

  • record.attachments|list in index-de.html records dependency on image-parent+de.lr and image-parent+en.lr (incorrect)
  • record.attachments|list in index-en.html records dependency on image-parent+en.lr (correct)

EDIT: also, I do not use attachement alternatives. This is a general problem.

@dairiki
Copy link
Contributor

dairiki commented Nov 24, 2022

Apologies for any incoherence. I've had a head-cold for the week and am not thinking all that clearly...

I just meant to point out that alt is available for attachments so that that Attachment.parent property that you quoted could be easily fixed to return a parent record of the correct (I think) alt.


I think this is probably, indeed a bug. There are probably other similar issues in the dependency tracking, as well, where unnecessary dependencies are being recorded.

(It's worth noting that recording unnecessary dependencies is a much more benign bug than missing a necessary dependency. The later can result in missed rebuilds and stale output artifacts, while the former only results in slower rebuilds.)


In the case of attachments, does the attachment need to declare a dependency on any of its parent's .lr files?

Can changes in any of those .lr files affect the "rendering" of the attachment? In the case of an attachment, I think, "rendering" means just copying to the correct location in the output tree. If that's the case, only files containing fields that can affect the computed URL of the attachment need to be listed as dependencies, right?

@relikd
Copy link
Contributor Author

relikd commented Nov 24, 2022

What about image croping and scaling? Will that be recorded by the template? For example, if the parent has a width property. Does that / should that creates a dependency on the parent or on the template only? Or how about image alt metadata, will that be a proper dependency? ... I am just shooting in the dark what the expected bahavior is.

Get well soon :)

@relikd
Copy link
Contributor Author

relikd commented Nov 25, 2022

This seems also to cause problems. It gets the parent with primary alt instead of currently used alt.
(in this case also, only to read the datamodel but a record dependency is created)

parent_obj = pad.get(parent)

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

Successfully merging a pull request may close this issue.

2 participants