Skip to content

Conversation

@alexbourret
Copy link
Contributor

@alexbourret alexbourret marked this pull request as draft May 21, 2025 12:50
@alexbourret alexbourret marked this pull request as ready for review June 3, 2025 09:23
@alexbourret alexbourret changed the base branch from feature/sc-241566-add-elementname-column-to-attribute-search to master September 22, 2025 14:24
@JaneBellaiche
Copy link

Tested with different sizes of streak and batch buffer

  • with 1 path <-> 1 value ✅
  • With 1 path <-> multiple values ✅
  • With multiple paths <-> 1 path par value ✅
  • With multiple paths <-> multiple values for each path ✅

}
# mark in self.responses that status of this write depends on result of streak X
if self.current_webid is None:
logger.info("webid now is {}".format(webid))

Choose a reason for hiding this comment

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

format a lil bit
also, do we want to show every webid?

Suggested change
logger.info("webid now is {}".format(webid))
logger.info("webid : {}".format(webid))

}
)
self.row_number += 1
logger.info("Pushed in buffer, now {}".format(len(self.streak_buffer)))

Choose a reason for hiding this comment

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

Could improve this log, it's not very clear what is after "now"

Copy link

@JaneBellaiche JaneBellaiche left a comment

Choose a reason for hiding this comment

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

LGTM 👍 Some logs are, I think, some rests of the helpers used to debug. Let's make sure the logs are clear and not too much redondant before merging the PR. Maybe, every X items in the buffer should be logged. Or just at the beginning and the end of the buffer. I let you find the best depending on what's needed for the client in term of log details and clarity

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 this pull request may close these issues.

3 participants