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.

Sign up for GitHub

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

Jump to bottom

Message #6808

Closed
busFred opened this issue Mar 25, 2024 · 3 comments
Closed

Message #6808

busFred opened this issue Mar 25, 2024 · 3 comments

Comments

@busFred
Copy link

busFred commented Mar 25, 2024

google.protobuf.json_format.MessageToJson() including_default_value_fields argument has been deprecated and removed since protobuf v5.26. need to use `always_print_fields_with_nopresence instead.

tensorboard/tensorboard/plugins/hparams/hparams_plugin.py

Line 148 in 71952d6

including_default_value_fields=True,

protocolbuffers/protobuf@2699579#diff-8de817c14d6a087981503c9aea38730b1b3e98f4e306db5ff9d525c7c304f234L89-L94
temporary fix is to just downgrade protobuf to 4.25
@DeepDuke
Copy link

@busFred TKS! Downgrading protobuf to 4.25 works.

@arcra
Copy link
Member

arcra commented Mar 28, 2024

Thanks for flagging this.

Looks like the new parameter was added less than 2 months ago in protocolbuffers/protobuf#15703, so it would only be present in very recent versions of protobuf dependencies, which might be a bit restrictive.

For now, I'll add a restriction for the protobuf dependency to be < 5.0.0, and eventually we can update it to use the new parameter when we need to bump this dependency again for some reason.

arcra added a commit to arcra/tensorboard that referenced this issue Mar 28, 2024
@arcra
Restricts the protobuf dependency to be < 5.0.0, due to a recent brea…
4d5610d 
…king change.

See tensorflow#6808 for details.
@arcra arcra mentioned this issue Mar 28, 2024
Merged
arcra added a commit that referenced this issue Mar 28, 2024
@arcra
Restricts the protobuf dependency to be < 5.0.0, due to a recent brea…
e795312 
…king change. (#6814)

The protobuf release 5.26.x is the first one in the major version 5,
which introduces a breaking change.

This change adds a restriction on that dependency. See #6808 for
details.
@arcra arcra closed this as completed Mar 28, 2024
marovira added a commit to marovira/helios-ml that referenced this issue Apr 5, 2024
@marovira
[brief] Restrict protobuf version to be less than 5.0.0
1449388 
[detailed]
- This issue is referenced in tensorflow/tensorboard#6808. It appears
  that protobuf deprecated an argument and tensorflow just restricted
  the version of protobuf to avoid issues. Since it's unclear when
  they're going to update, lets restrict the version ourselves to avoid
  complications.
AnuarTB pushed a commit to AnuarTB/tensorboard that referenced this issue May 3, 2024
@arcra @AnuarTB
Restricts the protobuf dependency to be < 5.0.0, due to a recent brea…
6b77522 
…king change. (tensorflow#6814)

The protobuf release 5.26.x is the first one in the major version 5,
which introduces a breaking change.

This change adds a restriction on that dependency. See tensorflow#6808 for
details.
@MuhongGuo MuhongGuo mentioned this issue May 14, 2024
Closed
@wimglenn
Copy link

@arcra Adding the upper bound like that is not really a great solution, because any reasonable solver with a constraint on protobuf>=5 will just go ahead and backtrack from tensorboard v2.17.0 to v2.16.2, and then remain broken.

A more compatible way to solve this is detect protobuf version and fix the call into json_format.MessageToJson conditionally depending on protobuf 4.x or 5.x.

@champialex champialex mentioned this issue Aug 7, 2024
Closed
groszewn added a commit to groszewn/tensorboard that referenced this issue Aug 7, 2024
@groszewn
Relax protobuf<5.0.0 restriction
6f41f86 
Instead of restricting protobuf<5.0.0 due to the removal to
`including_default_value_fields` kwarg in `json_format.MessageToJson`
(replaced by `always_print_fields_with_nopresence`), we can instead
detect the currently installed protobuf version and use the appropriate
kwarg.

Contributes to tensorflow#6808 and should resolve tensorflow#6887.
groszewn added a commit to groszewn/tensorboard that referenced this issue Aug 7, 2024
@groszewn
Relax protobuf<5.0.0 restriction
553f8f2 
Instead of restricting `protobuf<5.0.0` due to the removal to
`including_default_value_fields` kwarg in `json_format.MessageToJson`
(replaced by `always_print_fields_with_nopresence`), we can instead
detect the currently installed protobuf version and use the appropriate
kwarg.

Contributes to tensorflow#6808 and should resolve tensorflow#6887.
@groszewn groszewn mentioned this issue Aug 7, 2024
Merged
groszewn added a commit to groszewn/tensorboard that referenced this issue Aug 7, 2024
@groszewn
Relax protobuf<5.0.0 restriction
834efb9 
Instead of restricting `protobuf<5.0.0` due to the removal to
`including_default_value_fields` kwarg in `json_format.MessageToJson`
(replaced by `always_print_fields_with_nopresence`), we can instead
detect the currently installed protobuf version and use the appropriate
kwarg.

Contributes to tensorflow#6808 and should resolve tensorflow#6887.
groszewn added a commit to groszewn/tensorboard that referenced this issue Aug 7, 2024
@groszewn
Relax protobuf<5.0.0 restriction
de0d9c9 
Instead of restricting `protobuf<5.0.0` due to the removal to
`including_default_value_fields` kwarg in `json_format.MessageToJson`
(replaced by `always_print_fields_with_nopresence`), we can instead
detect the currently installed protobuf version and use the appropriate
kwarg.

Contributes to tensorflow#6808 and should resolve tensorflow#6887.
groszewn added a commit to groszewn/tensorboard that referenced this issue Aug 7, 2024
@groszewn
Relax protobuf<5.0.0 restriction
39460eb 
Instead of restricting `protobuf<5.0.0` due to the removal to
`including_default_value_fields` kwarg in `json_format.MessageToJson`
(replaced by `always_print_fields_with_nopresence`), we can instead
detect the currently installed protobuf version and use the appropriate
kwarg.

Contributes to tensorflow#6808 and should resolve tensorflow#6887.
groszewn added a commit to groszewn/tensorboard that referenced this issue Aug 9, 2024
@groszewn
Relax protobuf<5.0.0 restriction
118b30d 
Instead of restricting `protobuf<5.0.0` due to the removal to
`including_default_value_fields` kwarg in `json_format.MessageToJson`
(replaced by `always_print_fields_with_nopresence`), we can instead
detect the currently installed protobuf version and use the appropriate
kwarg.

Contributes to tensorflow#6808 and should resolve tensorflow#6887.
groszewn added a commit to groszewn/tensorboard that referenced this issue Aug 9, 2024
@groszewn
Relax protobuf<5.0.0 restriction
5edcb74 
Instead of restricting `protobuf<5.0.0` due to the removal to
`including_default_value_fields` kwarg in `json_format.MessageToJson`
(replaced by `always_print_fields_with_nopresence`), we can instead
detect the currently installed protobuf version and use the appropriate
kwarg.

Contributes to tensorflow#6808 and should resolve tensorflow#6887.
groszewn added a commit to groszewn/tensorboard that referenced this issue Aug 9, 2024
@groszewn
Relax protobuf<5.0.0 restriction
de4c2b1 
Instead of restricting `protobuf<5.0.0` due to the removal to
`including_default_value_fields` kwarg in `json_format.MessageToJson`
(replaced by `always_print_fields_with_nopresence`), we can instead
detect the currently installed protobuf version and use the appropriate
kwarg.

Contributes to tensorflow#6808 and should resolve tensorflow#6887.
groszewn added a commit to groszewn/tensorboard that referenced this issue Aug 9, 2024
@groszewn
Relax protobuf<5.0.0 restriction
5245f9b 
Instead of restricting `protobuf<5.0.0` due to the removal to
`including_default_value_fields` kwarg in `json_format.MessageToJson`
(replaced by `always_print_fields_with_nopresence`), we can instead
detect the currently installed protobuf version and use the appropriate
kwarg.

Contributes to tensorflow#6808 and should resolve tensorflow#6887.
groszewn added a commit to groszewn/tensorboard that referenced this issue Aug 9, 2024
@groszewn
Relax protobuf<5.0.0 restriction
d3d9271 
Instead of restricting `protobuf<5.0.0` due to the removal to
`including_default_value_fields` kwarg in `json_format.MessageToJson`
(replaced by `always_print_fields_with_nopresence`), we can instead
detect the currently installed protobuf version and use the appropriate
kwarg.

Contributes to tensorflow#6808 and should resolve tensorflow#6887.
groszewn added a commit that referenced this issue Aug 9, 2024
@groszewn
Relax protobuf<5.0.0 restriction (#6888)
0627ad5 
Instead of restricting `protobuf<5.0.0` due to the removal to
`including_default_value_fields` kwarg in `json_format.MessageToJson`
(replaced by `always_print_fields_with_nopresence`), we can instead
detect the currently installed protobuf version and use the appropriate
kwarg.

Contributes to #6808 and should resolve #6887.
groszewn added a commit to groszewn/tensorboard that referenced this issue Aug 13, 2024
@groszewn
Relax protobuf<5.0.0 restriction (tensorflow#6888)
9570874 
Instead of restricting `protobuf<5.0.0` due to the removal to
`including_default_value_fields` kwarg in `json_format.MessageToJson`
(replaced by `always_print_fields_with_nopresence`), we can instead
detect the currently installed protobuf version and use the appropriate
kwarg.

Contributes to tensorflow#6808 and should resolve tensorflow#6887.
groszewn added a commit that referenced this issue Aug 13, 2024
@groszewn
Relax protobuf<5.0.0 restriction (#6888)
9e6d7e8 
Instead of restricting `protobuf<5.0.0` due to the removal to
`including_default_value_fields` kwarg in `json_format.MessageToJson`
(replaced by `always_print_fields_with_nopresence`), we can instead
detect the currently installed protobuf version and use the appropriate
kwarg.

Contributes to #6808 and should resolve #6887.
marovira added a commit to marovira/helios-ml that referenced this issue Aug 29, 2024
@marovira
[brief] Remove the restriction on protobuf but increase tensorboard v…
0a457b8 
…ersion.

[detailed]
- tensorflow/tensorboard#6808 has been resolved, so we can now remove
  the explicit dependency to protobuf by bumping tensorboard to 2.17.1.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@arcra @wimglenn @DeepDuke @busFred