> On Oct. 31, 2014, 1:57 p.m., Jarek Cecho wrote: > > client/src/main/java/org/apache/sqoop/client/SqoopClient.java, line 85 > > <https://reviews.apache.org/r/27330/diff/7/?file=743043#file743043line85> > > > > I do feel that "SUBMITTED" and "STARTED" are two different cases. > > Submitted means that we've created the request to start a new job that will > > eventually transfer data. Started means that the job has started and is > > running now. > > > > This call will only create that request and will return before the > > actuall job will START. I don't see much different on our testing cluster > > as Submitted job is started pretty much immediately, however on real > > clusters the delay between submission and start can be significant as the > > job can be sitting in a scheduler queue for substantial amount of time > > before it starts (especially on busy clusters). > > Veena Basavaraj wrote: > there are subtle differences, submit and start are used in very different > ways. > > Think about this from the time we issue a command in the client, going by > your explanation the command should be submit. I am utterly confused reading > this code and how we have just used started and submitted without much > thought. > > I appreciate if we look at this from the client command / status to the > execution engine on what we want to call as submit and what should be start. > > Jarek Cecho wrote: > I think that the client and REST API have different audiences that have > different requirements. Most of the users don't know nor want to know the > diffence between Submit and Start. I feel that the fact that those are two > independent steps is pretty much an "implementation detail" from their > perspective. On the other hand the connector developper or someone who will > integrate with the REST API should know the difference and hence it's exposed > there. I believe that this is intentional. > > The project do have parts that are meant for different audiences and I > think that it's reasonable that different audiences are presented with > slightly different point of view. Simplification for users seems as good idea > whereas we should say accurate for someone who is extending Sqoop. > > Veena Basavaraj wrote: > correct me, I am not confused here. > > There is SqoopCLient -> so who is the audience for client. My > understanding is that anyone can use it. Not retreicted to developer. Using > rest/ client is a matter of preference is what I beleive. > > Second, I do not think different audiences need a different picture, it > just makes our coding and rationalizing what we do in sqoop code harder. > > Second, what is "most" mean? there are differences in clearly how we use > them. submit is like putting it on a queue for it to start. As you clealry > artiuclated it, it might start immediately or be delayed. There are error > messages when I use a start command,it says I cannot submit the job, what > does this indicate? > > Lets fix this. It might sound very trivial to you, but going forward it > will lead to more confusion if one another person joins this team and wants > his /her way. > > The rest api uses the SubmissionStatus I suppose directly and the SQoop > client uses its own enum. Why? > > Veena Basavaraj wrote: > see JIRA for details, taking this discussion to new a JIRA and new RB.
Thanks, make sense. > On Oct. 31, 2014, 1:57 p.m., Jarek Cecho wrote: > > core/src/main/java/org/apache/sqoop/repository/JdbcRepository.java, lines > > 508-520 > > <https://reviews.apache.org/r/27330/diff/7/?file=743062#file743062line508> > > > > This move seems unencessary? > > Veena Basavaraj wrote: > why? would you care to ask why I felt it is necessary? > > when reading code, having all the links related apis in one places makes > it wasy for some one to not miss the apis. Whats wrong with moving, you want > another RB for it? say that. > > Jus think about how you organize your table, when we write code lets do > the same, organize things so that at one shot we know what the apis for links > are. I would even want to split this up into logical classes as we have groen > the repository apis, see how hard it is to add post gres, wish we had thight > through this before. > > Jarek Cecho wrote: > Doing unnecessary moves is breaking "git blame" and "git cherrypick". For > easier readability we have all the methods and properties named in a way that > most generic part was at the begging and most specific one at the end. This > way all the overview tools (like method listing in IDEs) is showing similar > methods next to each other. > > Veena Basavaraj wrote: > I just gave an example. > > we call it updateLink, and not linkUpdate, what is the most generic par? > > Annd if your concern is about git blame, I will do another RB first with > these changes. I have no issues with it. > > Veena Basavaraj wrote: > Lets try to name methods like 90% of the people in other projects name > it, if we have these special rules in sqoop, I;d like to see it documented > and then we take a vote on why we need this. > > Until then I suggest we go back to what every one else names their > methods by. This is something we should start right now helping contributors > to be not confused on how to name a method and have a coherent story. > > First thing after this RB is shipped I will start the coding standard > wiki and cover these examples I'm all for specifying public code guidelines, let's create a separate JIRA for that? - Jarek ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27330/#review59331 ----------------------------------------------------------- On Nov. 3, 2014, 2:47 a.m., Veena Basavaraj wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/27330/ > ----------------------------------------------------------- > > (Updated Nov. 3, 2014, 2:47 a.m.) > > > Review request for Sqoop. > > > Repository: sqoop-sqoop2 > > > Description > ------- > > see JIRA and its parent JIRA for details > > FYI : it does nto include the start/submit and stop/abort renames. > > new tickets have been added to address these > > > Diffs > ----- > > client/src/main/java/org/apache/sqoop/client/SqoopClient.java 33a0c3c > client/src/main/java/org/apache/sqoop/client/SubmissionCallback.java > de7211a > > client/src/main/java/org/apache/sqoop/client/request/JobResourceRequest.java > 83c08b3 > > client/src/main/java/org/apache/sqoop/client/request/SqoopResourceRequests.java > 4a56bb7 > > client/src/main/java/org/apache/sqoop/client/request/SubmissionResourceRequest.java > 5055783 > common/src/main/java/org/apache/sqoop/json/JobBean.java 082d591 > common/src/main/java/org/apache/sqoop/json/JobsBean.java PRE-CREATION > common/src/main/java/org/apache/sqoop/json/JsonBean.java ba86511 > common/src/main/java/org/apache/sqoop/json/SubmissionBean.java 4b80338 > common/src/main/java/org/apache/sqoop/json/SubmissionsBean.java > PRE-CREATION > common/src/main/java/org/apache/sqoop/model/MSubmission.java 7290df5 > common/src/test/java/org/apache/sqoop/json/TestJobBean.java 1fc8dbd > common/src/test/java/org/apache/sqoop/json/TestSubmissionBean.java e4d50bf > core/src/main/java/org/apache/sqoop/driver/DriverError.java ddee282 > core/src/main/java/org/apache/sqoop/driver/JobManager.java ba56c77 > core/src/main/java/org/apache/sqoop/driver/JobRequest.java 2666320 > core/src/main/java/org/apache/sqoop/repository/JdbcRepository.java 976223d > core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java > 1e22759 > core/src/main/java/org/apache/sqoop/repository/Repository.java 09989e0 > > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java > b324f4f > > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaInsertUpdateDeleteSelectQuery.java > c894d06 > server/src/main/java/org/apache/sqoop/handler/JobRequestHandler.java > 5547988 > server/src/main/java/org/apache/sqoop/handler/LinkRequestHandler.java > 35a9635 > server/src/main/java/org/apache/sqoop/handler/SubmissionRequestHandler.java > 8555b0c > server/src/main/java/org/apache/sqoop/server/RequestHandler.java 508edd2 > server/src/main/java/org/apache/sqoop/server/v1/JobServlet.java d295237 > server/src/main/java/org/apache/sqoop/server/v1/JobsServlet.java > PRE-CREATION > server/src/main/java/org/apache/sqoop/server/v1/SubmissionServlet.java > 5c1d883 > server/src/main/java/org/apache/sqoop/server/v1/SubmissionsServlet.java > PRE-CREATION > server/src/main/webapp/WEB-INF/web.xml 6ad90d2 > shell/src/main/java/org/apache/sqoop/shell/ShowJobStatusFunction.java > PRE-CREATION > shell/src/main/java/org/apache/sqoop/shell/SqoopShell.java 2e87965 > shell/src/main/java/org/apache/sqoop/shell/StartJobFunction.java 4363f05 > shell/src/main/java/org/apache/sqoop/shell/StatusCommand.java 3447a87 > shell/src/main/java/org/apache/sqoop/shell/StatusJobFunction.java fb83af3 > shell/src/main/java/org/apache/sqoop/shell/StopJobFunction.java 790c522 > shell/src/main/java/org/apache/sqoop/shell/UpdateJobFunction.java dd075d7 > shell/src/main/java/org/apache/sqoop/shell/UpdateLinkFunction.java 176833a > test/src/main/java/org/apache/sqoop/test/testcases/ConnectorTestCase.java > 06462a3 > > test/src/test/java/org/apache/sqoop/integration/connector/jdbc/generic/FromRDBMSToHDFSTest.java > 36f7443 > tools/src/main/java/org/apache/sqoop/tools/tool/RepositoryDumpTool.java > c219e68 > > Diff: https://reviews.apache.org/r/27330/diff/ > > > Testing > ------- > > yes > > > Thanks, > > Veena Basavaraj > >
