Hadoop Common
  1. Hadoop Common
  2. HADOOP-10753

Could be better to move CommonConfigurationKeysPublic to org.apache.hadoop.common package

    Details

    • Type: Improvement Improvement
    • Status: Open
    • Priority: Minor Minor
    • Resolution: Unresolved
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: None
    • Labels:
      None

      Description

      As discussed in HADOOP-8943, it would be better to place CommonConfigurationKeysPublic into a more neutral place like org.apache.hadoop.common instead of org.apache.hadoop.fs package. This would help common facilities like security related codes avoid coupling with fs stuffs.

      Could anyone post your thought about this, and if confirmed, I would provide a patch for it.

        Activity

        Hide
        Jason Lowe added a comment -

        org.apache.hadoop.conf may be a more appropriate package.

        org.apache.hadoop.fs.CommonConfigurationKeysPublic is marked Public, therefore people are free to depend upon it. We can't simply move it because that break backwards compatibility. We may be able to get away with moving it then leaving a trivial derivation in org.apache.hadoop.fs via something like this:

        package org.apache.hadoop.fs;
        
        @InterfaceAudience.Public
        @Deprecated
        public class CommonConfigurationKeysPublic extends org.apache.hadoop.conf.CommonConfigurationKeysPublic {
        }
        
        Show
        Jason Lowe added a comment - org.apache.hadoop.conf may be a more appropriate package. org.apache.hadoop.fs.CommonConfigurationKeysPublic is marked Public, therefore people are free to depend upon it. We can't simply move it because that break backwards compatibility. We may be able to get away with moving it then leaving a trivial derivation in org.apache.hadoop.fs via something like this: package org.apache.hadoop.fs; @InterfaceAudience.Public @Deprecated public class CommonConfigurationKeysPublic extends org.apache.hadoop.conf.CommonConfigurationKeysPublic { }
        Hide
        Kai Zheng added a comment -

        Jason,

        I can't agree with you any more. I will do that. Thanks for your suggestion!
        Do you think it's better to change the references in current code base to use the new API of org.apache.hadoop.conf.CommonConfigurationKeysPublic?

        Show
        Kai Zheng added a comment - Jason, I can't agree with you any more. I will do that. Thanks for your suggestion! Do you think it's better to change the references in current code base to use the new API of org.apache.hadoop.conf.CommonConfigurationKeysPublic?
        Hide
        Steve Loughran added a comment -

        please don't mark the existing class as deprecated. It'll simply explode the amount of deprecated code in the builds. Anyone who is writing code to work with libraries prior to the move is going to need to stick to the old location until they stop.

        Maybe we should add a new InterfaceAudience tag, @InterfaceAudience.RetainedForCompatibility

        Show
        Steve Loughran added a comment - please don't mark the existing class as deprecated. It'll simply explode the amount of deprecated code in the builds. Anyone who is writing code to work with libraries prior to the move is going to need to stick to the old location until they stop. Maybe we should add a new InterfaceAudience tag, @InterfaceAudience.RetainedForCompatibility
        Hide
        Kai Zheng added a comment -

        Steve,

        Thanks for your thought.

        It'll simply explode the amount of deprecated code in the builds.

        Could this be avoided if we update the references in existing codes using the new API?

        A good idea, to add RetainedForCompatibility.

        Show
        Kai Zheng added a comment - Steve, Thanks for your thought. It'll simply explode the amount of deprecated code in the builds. Could this be avoided if we update the references in existing codes using the new API? A good idea, to add RetainedForCompatibility.

          People

          • Assignee:
            Kai Zheng
            Reporter:
            Kai Zheng
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:

              Development