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

fix: don't create logging handler multiple times (DEV-2891) #609

Merged
merged 17 commits into from Oct 30, 2023

Conversation

jnussbaum
Copy link
Collaborator

No description provided.

@linear
Copy link

linear bot commented Oct 27, 2023

DEV-2881 cleanup stashing algorithm

After implementation in production code (DEV-2), there are some things that should be cleaned up:

  • add logging / printing to new code

  • ~generate_upload_order()~~ should only have 2 return values~

    • ~stash_counter~~ can be calculated by the caller, with ~~sum(\[len(x) for x in stash_lookup.values()\])~
    • ~upload_order~~ doesn't make sense: once everything is stashed, there is no preferred order any more!~
  • ~generate_upload_order()~~ should be renamed to ~~generate_stash()~

@jnussbaum jnussbaum self-assigned this Oct 27, 2023
@jnussbaum jnussbaum marked this pull request as draft October 27, 2023 18:18
@jnussbaum jnussbaum changed the title chore: misc fix: don't create logging handler multiple times (DEV-2891) Oct 30, 2023
@linear
Copy link

linear bot commented Oct 30, 2023

DEV-2891 logging handler is created multiple times

Probably, this shouldn't happen. it would be better to create in on module level, so that it's created only once.

@jnussbaum jnussbaum marked this pull request as ready for review October 30, 2023 08:18
Copy link
Collaborator

@BalduinLandolt BalduinLandolt left a comment

Choose a reason for hiding this comment

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

Two minor comments, otherwise looking good. Did you notice any difference with only one handler created?

src/dsp_tools/utils/create_logger.py Outdated Show resolved Hide resolved
src/dsp_tools/utils/create_logger.py Outdated Show resolved Hide resolved
@jnussbaum jnussbaum enabled auto-merge (squash) October 30, 2023 12:56
@jnussbaum jnussbaum merged commit e6aa47c into main Oct 30, 2023
8 checks passed
@jnussbaum jnussbaum deleted the wip/DEV-2881-handle-errors-in-upload-files branch October 30, 2023 13:01
@daschbot daschbot mentioned this pull request Oct 30, 2023
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.

None yet

2 participants